GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Try to re-add stubs for ctypes by dgelessus · Pull Request #1906 · python/typeshed

Conversation

dgelessus commented Feb 20, 2018

Copy link

Copy Markdown

Contributor

Two years ago, some stubs for ctypes were added (#454) but removed soon afterwards, because they caused problems with correct code (#475). As I understand it, this was in part because of limitations in mypy at the time. Some of these limitations don't seem to exist anymore, so I tried to make the ctypes stubs work again.

I took the code in its last state before it was removed and tried to fix the issues reported in #475, as well as a few issues that I came across myself when using the stubs with my own code. Details on what exactly I've changed can be found in the individual commit messages. I'm sure there are still issues that I haven't noticed, so if anyone has a ctypes-using codebase and could try out these stubs, I would be happy to get some feedback.

Finally, I'd like to note that I don't have a lot of experience with mypy and writing typing stubs. Previously I've type-annotated some bits of my code and used typeshed through PyCharm's typechecker, but other than that this is my first time properly using mypy and typing. So if there's anything that I'm doing incorrectly or not according to convention, please let me know! 😃

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus added 14 commits

February 20, 2018 23:20


          Revert "Delete ctypes. It is not yet ready for prime time, alas."

acb456f

This reverts commit 73bdb70.


          Convert type comments to variable annotations in ctypes.__init__

acb3758


          Add missing List import in ctypes.__init__

45ac1a7


          Add missing bound kwarg to _CT TypeVar in ctypes.__init__

38fabd0


          Fix ctypes._CData classmethod definitions

265a831


          Fix ctypes Structure/Union __getattr__ definitions

2bb0fe9

Both Structure's metaclass and Structure itself define a __getattr__.
The former returns field objects (each representing a field definition
in the Structure), while the latter can return anything (the value of
the field in a specific Structure instance).
Structure also defines a __setattr__, while Structure's metaclass
does not.

BigEndianStructure, LittleEndianStructure and Union support the same
operations as Structure does, but the semantics obviously differ.

Depending on the system endianness, exactly one of the EndianStructures
is an alias for Structure, and the other one is a special Structure
subclass. For simplicity, both EndianStructures are considered
subclasses of Structure here, even though this is technically not
always accurate.


          Fix ctypes._CData classmethod definitions properly this time

a73cd5e

The classmethods return *instances* of the class of course, not the
class itself.


          Fix swapped parameter types in ctypes._CData.in_dll


          Add limited support for ctypes._CData.__class__.__mul__

0dc881a

It's not possible to specify the correct return type for __mul__ and
__rmul__ here - see comments for explanation.


          Make ctypes._FuncPtr extend ctypes._CData

ba2a9e5


          Improve typing of ctypes.cast

aabc546


          Mark class attributes with ClassVar in ctypes.__init__


          Mark ctypes._CData.from_buffer[_copy] offset arg as optional


          Remove trailing whitespace in ctypes.__init__

50e44c9

JelleZijlstra requested changes

Mar 6, 2018

View reviewed changes

JelleZijlstra left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for adding this, and sorry for the wait before the review!

I left a couple of comments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated

		Any, Callable, ClassVar, Iterable, List, Mapping, Optional, Sequence, Sized, Tuple, Type,
		Generic, TypeVar, overload,
		)
		from typing import Union as UnionT

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You should name this _UnionT so it doesn't get re-exported. (With the current definition, mypy would let users write ctypes.UnionT.)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated

		class _DLL:
		def __init__(self, name: str, mode: int = ..., handle: Optional[int] = ...,
		use_errno: bool = ..., use_last_error: bool = ...) -> None: ...
		def __getattr__(self, name: str) -> Any: ...

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks like there's also __getitem__.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated

		class PyDLL(_DLL):
		_handle: int = ...
		_name: str = ...
		def __init__(self, name: str, mode: int = ...,

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Isn't the __init__ just inherited from the base class? https://github.com/python/cpython/blob/2.7/Lib/ctypes/__init__.py#L389

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus Mar 6, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Technically yes, but the documentation lists the constructor as not accepting the use_errno and use_last_error kwargs. I don't know what the correct procedure is in cases like this - is the documentation or the code right?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

JelleZijlstra Mar 7, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'd generally favor the code.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus Mar 7, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

OK, I've removed the __init__ from PyDLL, it should now inherit the one from CDLL again.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi


		class LibraryLoader(Generic[_DLLT]):
		def __init__(self, dlltype: Type[_DLLT]) -> None: ...
		def LoadLibrary(self, name: str) -> _DLLT: ...

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Missing __getattr__ and __getitem__.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated

		def create_string_buffer(init_or_size: UnionT[int, bytes],
		size: Optional[int] = ...) -> Array[c_char]: ...
		c_buffer = create_string_buffer
		def create_unicode_buffer(init_or_size: UnionT[int, str],

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should be Text for Python 2. The actual code checks for (str, unicode).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated


		class c_byte(_SimpleCData[int]): ...

		class c_char(_SimpleCData[bytes]): ...

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The __init__ argument here can also be an int.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi

		class c_byte(_SimpleCData[int]): ...

		class c_char(_SimpleCData[bytes]): ...
		class c_char_p(_PointerLike, _SimpleCData[Optional[bytes]]):

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Why is it Optional? If the value is inaccessible, it just segfaults (as I just found out experimentally).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus Mar 6, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Not here - the value of a null pointer is None.

Python 3.6.4 (v3.6.4:d48ecebad5, Dec 18 2017, 21:07:28) 
[GCC 4.2.1 (Apple Inc. build 5666) (dot 3)] on darwin
>>> import ctypes
>>> ccp = ctypes.c_char_p(None)
>>> ccp.value
>>> cwp = ctypes.c_wchar_p(None)
>>> cwp.value

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

JelleZijlstra reacted with thumbs up emoji

All reactions

👍 1 reaction

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated


		class c_void_p(_PointerLike, _SimpleCData[Optional[int]]): ...

		class c_wchar(_SimpleCData[str]): ...

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Should this be Text?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

stdlib/2and3/ctypes/__init__.pyi Outdated

		if sys.platform == 'win32':
		class HRESULT(_SimpleCData[Any]): ... # TODO undocumented

		class py_object(Generic[_T], _SimpleCData[_T]): ...

JelleZijlstra Mar 6, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Don't need the explicit Generic base class.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus added 15 commits

March 6, 2018 11:32


          Don't export ctypes.UnionT

535a008


          Add ctypes._DLL.__getitem__

aedd0d9


          Make ctypes._DLL.__get(attr|item)__ return _FuncPtr instead of Any

2311ced


          Change ctypes DLL inheritance hierarchy to match the real one better

854e22e


          Add some missing attributes to ctypes.CDLL

e765dec


          Rename ctypes._FuncPtr so it doesn't conflict with CDLL._FuncPtr

7a34b64


          Fix type of ctypes.CDLL._FuncPtr


          Merge _FuncProto into _FuncPointer

94a9853

The function pointer types returned by CFUNCTYPE and friends are the
same as those encountered elsewhere, so there's no need to treat them
differently.


          Fix some leftover references to ctypes._DLL


          Simplify definition of ctypes._DLLT

253fc4a


          Add ctypes.LibraryLoader.__get(attr|item)__

cc3f49a


          Use Text instead of str where appropriate in ctypes.__init__

7ff6017


          Make ctypes.c_char accept ints

66a74a8


          Make ctypes.c_[w]char_p accept None

e5d1642


          Remove unneeded Generic base from ctypes.py_object

b81de47


          Make ctypes.cast accept _cparam

a7507ea

dgelessus commented Mar 6, 2018

Copy link

Copy Markdown

Contributor Author

OK, should all be fixed (along with a couple of other issues that I noticed/remembered).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

dgelessus added 2 commits

March 6, 2018 13:41


          Fix ctypes._PF being declared too late

d8d431e


          Remove incorrect ctypes.PyDLL.__init__ override

a38f579

JelleZijlstra approved these changes

Mar 17, 2018

View reviewed changes

JelleZijlstra merged commit 8009a34 into python:master

Mar 17, 2018

eric-wieser reviewed

Nov 19, 2018

View reviewed changes

Comment thread

stdlib/2and3/ctypes/__init__.pyi

		@classmethod
		def from_buffer(cls: Type[_CT], source: bytearray, offset: int = ...) -> _CT: ...
		@classmethod
		def from_buffer_copy(cls: Type[_CT], source: bytearray, offset: int = ...) -> _CT: ...

eric-wieser Nov 19, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Shouldn't this be bytes not bytearray?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus Nov 19, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Both of the bytearray types are inadequate actually - from_buffer_copy accepts any object that implements the buffer protocol, and similarly from_buffer accepts any writable buffer. bytearray and bytes are common examples of read-write and read-only buffer objects, but there are others like memoryview and array.array. All ctypes data objects (_CData subclasses) are writable buffers as well.

Does typeshed/typing have a type for "anything implementing the buffer protocol"? If not, we probably need to use Unions and list the most common stdlib buffer types.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

eric-wieser reacted with thumbs up emoji

All reactions

👍 1 reaction

ilevkivskyi Nov 19, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

You can define a new protocol. I think there are already many examples in typeshed (grep for Protocol).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ilevkivskyi Nov 19, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Oh, it is C-level, in this case looks like a big union is the only option.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

eric-wieser Nov 20, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Isn't bytes already treated by mypy as anything implementing the buffer protocol?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

srittau Nov 20, 2018

Copy link

Copy Markdown

Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This is what the typing documentation says:

This type [ByteString] represents the types bytes, bytearray, and memoryview.

As a shorthand for this type, bytes can be used to annotate arguments of any of the types mentioned above.

This should probably be added to PEP 484.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus Nov 20, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fix here: #2610

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

dgelessus added a commit to dgelessus/typeshed that referenced this pull request

Nov 20, 2018


          Let ctypes._CData.from_buffer[_copy] accept any buffer object

9a4bfe9

Fixes the issue discussed here:
python#1906 (comment)

dgelessus mentioned this pull request

Nov 20, 2018

Let ctypes._CData.from_buffer[_copy] accept any buffer object #2610

Merged

JelleZijlstra pushed a commit that referenced this pull request

Nov 20, 2018


          Let ctypes._CData.from_buffer[_copy] accept any buffer object (#2610)

45d4e88

Fixes the issue discussed here:
#1906 (comment)

yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this pull request

Jan 23, 2019


          Re-add stubs for ctypes (python#1906)

48539e7

* Convert type comments to variable annotations in ctypes.__init__

* Add missing List import in ctypes.__init__

* Add missing bound kwarg to _CT TypeVar in ctypes.__init__

* Fix ctypes._CData classmethod definitions

* Fix ctypes Structure/Union __getattr__ definitions

Both Structure's metaclass and Structure itself define a __getattr__.
The former returns field objects (each representing a field definition
in the Structure), while the latter can return anything (the value of
the field in a specific Structure instance).
Structure also defines a __setattr__, while Structure's metaclass
does not.

BigEndianStructure, LittleEndianStructure and Union support the same
operations as Structure does, but the semantics obviously differ.

Depending on the system endianness, exactly one of the EndianStructures
is an alias for Structure, and the other one is a special Structure
subclass. For simplicity, both EndianStructures are considered
subclasses of Structure here, even though this is technically not
always accurate.

* Fix swapped parameter types in ctypes._CData.in_dll

* Add limited support for ctypes._CData.__class__.__mul__

It's not possible to specify the correct return type for __mul__ and
__rmul__ here - see comments for explanation.

* Make ctypes._FuncPtr extend ctypes._CData

* Improve typing of ctypes.cast

* Mark class attributes with ClassVar in ctypes.__init__

* Mark ctypes._CData.from_buffer[_copy] offset arg as optional

* Remove trailing whitespace in ctypes.__init__

* Don't export ctypes.UnionT

* Add ctypes._DLL.__getitem__

* Make ctypes._DLL.__get(attr|item)__ return _FuncPtr instead of Any

* Change ctypes DLL inheritance hierarchy to match the real one better

* Add some missing attributes to ctypes.CDLL

* Rename ctypes._FuncPtr so it doesn't conflict with CDLL._FuncPtr

* Fix type of ctypes.CDLL._FuncPtr

* Merge _FuncProto into _FuncPointer

The function pointer types returned by CFUNCTYPE and friends are the
same as those encountered elsewhere, so there's no need to treat them
differently.

* Fix some leftover references to ctypes._DLL

* Simplify definition of ctypes._DLLT

* Add ctypes.LibraryLoader.__get(attr|item)__

* Use Text instead of str where appropriate in ctypes.__init__

* Make ctypes.c_char accept ints

* Make ctypes.c_[w]char_p accept None

* Remove unneeded Generic base from ctypes.py_object

* Make ctypes.cast accept _cparam

* Fix ctypes._PF being declared too late

* Remove incorrect ctypes.PyDLL.__init__ override

yedpodtrzitko pushed a commit to yedpodtrzitko/typeshed that referenced this pull request

Jan 23, 2019


          Let ctypes._CData.from_buffer[_copy] accept any buffer object (python…

69f0702

…#2610)

Fixes the issue discussed here:
python#1906 (comment)

Reviewers

srittau

srittau left review comments

ilevkivskyi

ilevkivskyi left review comments

JelleZijlstra

JelleZijlstra approved these changes

+1 more reviewer

eric-wieser

eric-wieser left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.