Skip to content

Conversation

@AlexWaygood
Copy link
Member

Fixes#8351. Fixes#7747. Closes#7753.

@AlexWaygoodAlexWaygood marked this pull request as draft July 30, 2022 16:50
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

psycopg (https://github.com/psycopg/psycopg) + psycopg/psycopg/pq/_pq_ctypes.pyi:13: error: Unused "type: ignore" comment+ psycopg/psycopg/pq/_pq_ctypes.pyi:13: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:13: note: Error code "valid-type" not covered by "type: ignore" comment+ psycopg/psycopg/pq/_pq_ctypes.pyi:13: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:65: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:65: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:72: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:72: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:113: error: Unused "type: ignore" comment+ psycopg/psycopg/pq/_pq_ctypes.pyi:113: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:113: note: Error code "valid-type" not covered by "type: ignore" comment+ psycopg/psycopg/pq/_pq_ctypes.pyi:113: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:136: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:136: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:161: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:161: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:185: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:185: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:186: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:186: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:187: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:187: note: Perhaps you need "Callable[...]" or a callback protocol?+ psycopg/psycopg/pq/_pq_ctypes.pyi:189: error: Function "ctypes.pointer" is not valid as a type [valid-type]+ psycopg/psycopg/pq/_pq_ctypes.pyi:189: note: Perhaps you need "Callable[...]" or a callback protocol?

@AlexWaygoodAlexWaygood marked this pull request as ready for review July 30, 2022 16:58
@AlexWaygood
Copy link
MemberAuthor

As expected, the "moral" thing to do here is quite disruptive. @srittau, what do you think? Worth it anyway?

@AlexWaygood
Copy link
MemberAuthor

AlexWaygood commented Jul 30, 2022

As expected, the "moral" thing to do here is quite disruptive.

Having said that, it looks like the psycopg stubs which mypy is erroring on here are autogenerated: https://github.com/psycopg/psycopg/blob/cace92a9854130da02e55f416b2bcbefdc7242e1/psycopg/psycopg/pq/_pq_ctypes.py#L727.

So the fix for that library would be pretty trivial, I think.

Copy link
Collaborator

@srittausrittau left a comment

Choose a reason for hiding this comment

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

Let's do this. It might hurt now, but will probably prevent more hurt in the future.

@srittausrittau merged commit 87fc724 into masterJul 31, 2022
@srittausrittau deleted the pointer branch July 31, 2022 13:29
@AlexWaygood
Copy link
MemberAuthor

@dvarrazzo, just to give you a heads-up about this — the typeshed stubs for ctypes have been a bit of a hack for a while. This PR brings them closer to the runtime reality, but it means you might have to change your stubs for psycopg (after a future release of mypy) to use ctypes._Pointer instead of ctypes.pointer.

@dvarrazzo
Copy link

@AlexWaygood no problem at all! When this change will be released I'll be happy to update our stubs and set a dependency on the newer mypy version 🙂

@AlexWaygoodAlexWaygood mentioned this pull request Aug 11, 2022
@nathanpage-credo
Copy link

nathanpage-credo commented Aug 12, 2022

As @AlexWaygood mentioned, this breaks things where a user may want to define pointer annotations:

importctypesasctclassFoo(ct.Structure): ... # other stuff defined here test: "ct.pointer[ct.c_int32]"# these now errortest2: "ct.pointer[ct.c_bool]"# these now error

Could ctypes._Pointer be a non-internal type (e.g., ctypes.Pointer)?

@JelleZijlstra
Copy link
Member

Could ctypes._Pointer be a non-internal type (e.g., ctypes.Pointer)?

Maybe, but that's not a decision we can make in typeshed. If you want it to be, you should open an issue over at CPython.

@nathanrpage97
Copy link

Ok, I don’t think it is worthy of a change to cpython implementation. The big thing is linters not liking a reference to a private api. I think for now I will re-export ctypes._Pointer -> PointerType to reduce linter silencing needed.

@Akuli
Copy link
Collaborator

Cpython might accept making it a public type, a bit similarly to how _curses.window is now exposed as curses.window.

@AlexWaygood
Copy link
MemberAuthor

Cpython might accept making it a public type, a bit similarly to how _curses.window is now exposed as curses.window.

Especially given that ctypes._Pointer is actually documented -- it seems like it's not really meant to be a "private" class.

Sign up for freeto 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.

Typing of ctypes' pointer types is wrong mypy error: Name "ctypes._Pointer" is not defined

8 participants

@AlexWaygood@dvarrazzo@nathanpage-credo@JelleZijlstra@nathanrpage97@Akuli@srittau