Skip to content

The rest #92

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 28, 2020
Merged

The rest #92

merged 11 commits into from
Sep 28, 2020

Conversation

altendky
Copy link
Collaborator

No description provided.

@@ -6220,7 +6221,7 @@ class QMetaType(sip.simplewrapper):

def __init__(self, type: int = ...) -> None: ...

def name(self) -> QByteArray: ...
def name(self) -> QtCore.QByteArray: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -473,7 +473,7 @@ class QPyDesignerTaskMenuExtension(QtCore.QObject, QDesignerTaskMenuExtension):
def __init__(self, parent: QtCore.QObject) -> None: ...


class QPyDesignerPropertySheetExtension(QtCore.QObject, QDesignerPropertySheetExtension):
class QPyDesignerPropertySheetExtension(QtCore.QObject, QDesignerPropertySheetExtension): # type: ignore[misc]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyQt5-stubs/QtDesigner.pyi:476: error: Definition of "property" in base class "QObject" is incompatible with definition in base class "QDesignerPropertySheetExtension"  [misc]
PyQt5-stubs/QtDesigner.pyi:476: error: Definition of "setProperty" in base class "QObject" is incompatible with definition in base class "QDesignerPropertySheetExtension"  [misc]

@@ -103,7 +103,7 @@ class QQuickItem(QtCore.QObject, QtQml.QQmlParserStatus):
@typing.overload
def __init__(self, v: float) -> None: ...
@typing.overload
def __init__(self, v: bool) -> None: ...
def __init__(self, v: bool) -> None: ... # type: ignore[misc]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyQt5-stubs/QtQuick.pyi:106: error: Overloaded function signature 4 will never be matched: signature 3's parameter type(s) are the same or broader  [misc]

@@ -41,7 +41,7 @@ Buffer = Union['array', 'voidptr', str, bytes, bytearray]


# The array type.
class array(Sequence): ...
class array(Sequence): ... # type: ignore[misc]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyQt5-stubs/sip.pyi:44: error: Class PyQt5-stubs.sip.array has abstract attributes "__getitem__", "__len__"  [misc]
PyQt5-stubs/sip.pyi:44: note: If it is meant to be abstract, add 'abc.ABCMeta' as an explicit metaclass

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do as the error suggests and make array abstract?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or to add the missing abstract members to the stub class?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... I was trying to dig further and PyQt5.sip.array doesn't even seem to exist. I haven't managed to create an instance yet either by trying to call methods supposedly returning one. I was hoping I could both explore it and figure out where it comes from. Though for all I know it just gets converted to a list anyways... So I'm not clear how to decide what the best option is here.

>>> from PyQt5 import sip
>>> sip.array
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: module 'PyQt5.sip' has no attribute 'array'
>>> print('\n'.join(dir(sip)))
SIP_VERSION
SIP_VERSION_STR
_C_API
__doc__
__file__
__loader__
__name__
__package__
__spec__
_unpickle_enum
_unpickle_type
assign
cast
delete
dump
enableautoconversion
enableoverflowchecking
getapi
isdeleted
ispycreated
ispyowned
setapi
setdeleted
setdestroyonexit
settracemask
simplewrapper
transferback
transferto
unwrapinstance
voidptr
wrapinstance
wrapper
wrappertype

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
I found this in the SIP documentation.

returns the sip.array object

I feel like this is important, but I know next to nothing about SIP:

sip.array objects are not supported by the sip5 code generator. They can only be created by handwritten code or by sip.voidptr.asarray().

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes it sound like a non-abstract thing... maybe? It takes a len parameter so you'd think you could len() it which would mean it'd have .__len__(). Probably? And an array isn't much of an array without being able to index it so .__getitem__() seems reasonable. I think I'm on board with adding those. Just a sec...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I was able to make a sip.array object using this as reference:

import struct
from PyQt5 import QtCore

shared_mem = QtCore.QSharedMemory()
shared_mem.setNativeKey("test")
shared_mem.create(4*6)
shared_mem.attach()

memtocopy = [0,1,2,3,4,5]

shared_mem.lock()
shared_mem.data()[:] = memoryview(struct.pack('=6i', *memtocopy))
shared_mem.unlock()

array = shared_mem.data().asarray()
print(type(array))
<class 'sip.array'>

Copy link
Collaborator

@BryceBeagle BryceBeagle Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> len(array)
24

>>> array[20]
5

>>> "".join(map(str, array))
000010002000300040005000

Very strange, but it looks like both __len__ and __getitem__ are implemented

Copy link
Collaborator

@BryceBeagle BryceBeagle Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reason mypy complains that it should be abstract is because collections.abc.Sequence is abstract. This means we should probably add annotations for __len__, _getitem__, etc

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> dir(array)
['__class__', '__delattr__', '__delitem__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__getitem__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__len__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__setitem__', '__sizeof__', '__str__', '__subclasshook__']

@altendky
Copy link
Collaborator Author

Sorry... :|

@@ -41,7 +41,7 @@ Buffer = Union['array', 'voidptr', str, bytes, bytearray]


# The array type.
class array(Sequence): ...
class array(Sequence): ... # type: ignore[misc]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do as the error suggests and make array abstract?

@altendky
Copy link
Collaborator Author

/me glances at the ... _green_ CI check

Welp, almost time to increase mypy's pickiness and start using stubtest and add more checks and...

@@ -44,7 +44,9 @@ T = TypeVar("T")


# The array type.
class array(Sequence[T]): ...
class array(Sequence[T]):
def __getitem__(self, params: int) -> T: ...
Copy link
Collaborator Author

@altendky altendky Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://mypy-play.net/?mypy=latest&python=3.8&gist=b4c189ca505b6875dc41427d707ccd06

from typing import Any, Sequence, TypeVar


T = TypeVar("T")


class array(Sequence[T]):
    def __getitem__(self, key: Any) -> Any: ...
    def __len__(self) -> int: ...


class again(Sequence[T]):
    def __getitem__(self, key: int) -> Any: ...
    def __len__(self) -> int: ...


class more(Sequence[T]):
    def __getitem__(self, key: Any) -> T: ...
    def __len__(self) -> int: ...
main.py:13: error: Signature of "__getitem__" incompatible with supertype "Sequence"
main.py:18: error: Signature of "__getitem__" incompatible with supertype "Sequence"
Found 2 errors in 1 file (checked 1 source file)

ugh... have to just Any both the index and the result? What in the world is the use of this being a Generic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naw, I'm getting somewhere better. In progress...

@overload
def __getitem__(self, key: int) -> T: ...
@overload
def __getitem__(self: C, key: slice) -> C: ...
Copy link
Collaborator

@BryceBeagle BryceBeagle Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the slice one be

def __getitem__(self, key: slice) -> array[T]: ...

Copy link
Collaborator

@BryceBeagle BryceBeagle Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that generic self is for returning a modified version of the same object, a la builder pattern

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this more flexible for inheritance while still hinting the same thing for a sip.array itself? But sure, even if I am correct, that is at least unlikely to matter.

Copy link
Collaborator

@BryceBeagle BryceBeagle Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm not too sure how to get covariant return typing working with something that's already Generic. Your solution may be more correct

Copy link
Collaborator Author

@altendky altendky Sep 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So typeshed just uses covariant all over typing.Sequence.

https://github.com/python/typeshed/blob/734d91f90bee3c782aa6c7c04492882dbc3c47b1/stdlib/3/typing.pyi#L292-L298

class Sequence(_Collection[_T_co], Reversible[_T_co], Generic[_T_co]):
    @overload
    @abstractmethod
    def __getitem__(self, i: int) -> _T_co: ...
    @overload
    @abstractmethod
    def __getitem__(self, s: slice) -> Sequence[_T_co]: ...

I'm still taking passes trying to get variance internalized well. I suppose I'll just go with that (minus abstract and with array not Sequence), but I am curious how C would get it wrong. I would expect it to pass everything through. Any chance you have any reference for the idea that the C approach relates to returning self?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I meant to ask this earlier. Do we need to annotate __setitem__?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could fill out the help() entries anyways.

class array(builtins.object)
 |  Methods defined here:
 |  
 |  __delitem__(self, key, /)
 |      Delete self[key].
 |  
 |  __getitem__(self, key, /)
 |      Return self[key].
 |  
 |  __len__(self, /)
 |      Return len(self).
 |  
 |  __setitem__(self, key, value, /)
 |      Set self[key] to value.

Copy link
Collaborator

@BryceBeagle BryceBeagle Sep 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah not trying to make this PR drag on forever... but there's also a slice overload for __setitem__ to deal with 😖

__delitem__ has one too, but I tried using it and got a segfault. That may be just me doing something wrong with the shared memory, though

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know... I did think about that... just enough to forget it. And yeah, I knew when I put five changes in one that something would hold them up. :]

Per `help()`

```
class array(builtins.object)
 |  Methods defined here:
 |  
 |  __delitem__(self, key, /)
 |      Delete self[key].
 |  
 |  __getitem__(self, key, /)
 |      Return self[key].
 |  
 |  __len__(self, /)
 |      Return len(self).
 |  
 |  __setitem__(self, key, value, /)
 |      Set self[key] to value.
```
Comment on lines +48 to +51
@overload
def __delitem__(self, key: int) -> None: ...
@overload
def __delitem__(self, key: slice) -> None: ...
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a Union but I figured consistency would be just as well.

@altendky
Copy link
Collaborator Author

*sigh*

PyQt5-stubs/sip.pyi:58: error: Cannot use a covariant type variable as a parameter
PyQt5-stubs/sip.pyi:60: error: Name 'Iterable' is not defined
PyQt5-stubs/sip.pyi:60: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Iterable")
Found 2 errors in 1 file (checked 46 source files)

@altendky
Copy link
Collaborator Author

So typeshed just uses object for the parameter... (using .contains() as the reference) Though .index() uses Any. So I guess I'll have to look around unless you know offhand.

https://github.com/python/typeshed/blob/734d91f90bee3c782aa6c7c04492882dbc3c47b1/stdlib/3/typing.pyi#L292-L304

class Sequence(_Collection[_T_co], Reversible[_T_co], Generic[_T_co]):
    @overload
    @abstractmethod
    def __getitem__(self, i: int) -> _T_co: ...
    @overload
    @abstractmethod
    def __getitem__(self, s: slice) -> Sequence[_T_co]: ...
    # Mixin methods
    def index(self, value: Any, start: int = ..., stop: int = ...) -> int: ...
    def count(self, value: Any) -> int: ...
    def __contains__(self, x: object) -> bool: ...
    def __iter__(self) -> Iterator[_T_co]: ...
    def __reversed__(self) -> Iterator[_T_co]: ...

@altendky
Copy link
Collaborator Author

For that matter, should this be a MutableSequence? I guess not since it doesn't implement everything required but... As a reference point, MutableSequence doesn't use a covariant TypeVar at all.

https://github.com/python/typeshed/blob/734d91f90bee3c782aa6c7c04492882dbc3c47b1/stdlib/3/typing.pyi#L306-L326

class MutableSequence(Sequence[_T], Generic[_T]):
    @abstractmethod
    def insert(self, index: int, value: _T) -> None: ...
    @overload
    @abstractmethod
    def __getitem__(self, i: int) -> _T: ...
    @overload
    @abstractmethod
    def __getitem__(self, s: slice) -> MutableSequence[_T]: ...
    @overload
    @abstractmethod
    def __setitem__(self, i: int, o: _T) -> None: ...
    @overload
    @abstractmethod
    def __setitem__(self, s: slice, o: Iterable[_T]) -> None: ...
    @overload
    @abstractmethod
    def __delitem__(self, i: int) -> None: ...
    @overload
    @abstractmethod
    def __delitem__(self, i: slice) -> None: ...
<snip>

@altendky
Copy link
Collaborator Author

@BryceBeagle, care to reapprove now that i completely changed to invariant...?

@BryceBeagle
Copy link
Collaborator

For that matter, should this be a MutableSequence? I guess not since it doesn't implement everything required but... As a reference point, MutableSequence doesn't use a covariant TypeVar at all.

Yeah I don't think array supports the .insert that MutableSequence adds.

@altendky altendky merged commit 44aa344 into python-qt-tools:master Sep 28, 2020
@altendky altendky deleted the the_rest branch September 28, 2020 22:05
altendky added a commit to altendky/PyQt5-stubs that referenced this pull request Oct 2, 2020
bluebird75 pushed a commit to bluebird75/PyQt5-stubs that referenced this pull request Aug 24, 2021
bluebird75 pushed a commit to bluebird75/PyQt5-stubs that referenced this pull request Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants