Skip to content

Makes Array abstract #6361

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 5 commits into from
Nov 28, 2021
Merged

Makes Array abstract #6361

merged 5 commits into from
Nov 28, 2021

Conversation

sobolevn
Copy link
Member

Now, this code:

from ctypes import Array, c_int

class I(Array[c_int]):
    ...

Array()
I()

Raises:

out/ex.py:6: error: Cannot instantiate abstract class "Array" with abstract attributes "_length_" and "_type_"
out/ex.py:7: error: Cannot instantiate abstract class "I" with abstract attributes "_length_" and "_type_"

Tested locally with recent mypy.

Refs #6349

@sobolevn
Copy link
Member Author

sobolevn commented Nov 23, 2021

@github-actions

This comment has been minimized.

@sobolevn
Copy link
Member Author

I have to support _type_: str as well. Like here

Or not. It does not work in runtime. There are just some confusing examples in the CPython's source code.

@AlexWaygood
Copy link
Member

This feels like it maybe relies on a mypy bug at the moment. Adding abstractmethod to a class's methods shouldn't make the class abstract unless the class uses an ABCMeta-derived as its metaclass, and ctypes.Array doesn't do that in the stubs at the moment. The fact that mypy nonetheless recognises ctypes.Array as abstract, even though it doesn't have an ABCMeta-derived class as its metaclass, feels like something of a bug in mypy 🙂

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@JukkaL
Copy link
Contributor

JukkaL commented Nov 23, 2021

The fact that mypy nonetheless recognises ctypes.Array as abstract, even though it doesn't have an ABCMeta-derived class as its metaclass, feels like something of a bug in mypy

This is by design. It allows defining ABCs that are only checked during static type checking but not at runtime. This can be helpful to avoid metaclass conflicts, for example.

@AlexWaygood
Copy link
Member

This is by design. It allows defining ABCs that are only checked during static type checking but not at runtime. This can be helpful to avoid metaclass conflicts, for example.

Thanks, TIL!

@sobolevn
Copy link
Member Author

sobolevn commented Nov 23, 2021

Any ideas how can we fix this pytype problem? https://github.com/python/typeshed/runs/4297765776?check_suite_focus=true (Sorry, I've never used pytype)

stdlib/ctypes/__init__.pyi (3.9): ParseError: Overloaded signatures for _length_ disagree on abstractmethod decorators

First idea

This does not work with mypy:

    @property
    @abstractmethod
    def _length_(self) -> int: ...
    @_length_.setter
    @abstractmethod
    def _length_(self, value: int) -> None: ...

    @property
    @abstractmethod
    def _type_(self) -> type: ...
    @_type_.setter
    @abstractmethod
    def _type_(self, value: type) -> None: ...
out/ex.py:4: error: Overloaded method has both abstract and non-abstract variants
out/ex.py:7: error: Decorated property not supported
out/ex.py:11: error: Overloaded method has both abstract and non-abstract variants
out/ex.py:14: error: Decorated property not supported

This does not seem right to me: looks like this code should be accepted by mypy:

  1. It works fine in runtime
  2. It is documented here: https://docs.python.org/3/library/abc.html#abc.abstractproperty

I will open a new issue for it: python/mypy#11601

@Akuli
Copy link
Collaborator

Akuli commented Nov 23, 2021

CC @rchen152

@rchen152
Copy link
Collaborator

Huh, looks like the issue might be that pytype thinks that the property and setter methods are overloads? I'll take a look.

@rchen152
Copy link
Collaborator

I've sent a fix for the pytype issue out for review. Hopefully, I'll be able to do a release tomorrow; if not, it'll have to wait until next Monday (after Thanksgiving).

rchen152 added a commit to google/pytype that referenced this pull request Nov 24, 2021
@rchen152
Copy link
Collaborator

I've released pytype-2021.11.24, which should fix the pytype_test failure.

@sobolevn
Copy link
Member Author

@rchen152 thank you! 🎉

@sobolevn sobolevn mentioned this pull request Nov 25, 2021
hauntsaninja pushed a commit that referenced this pull request Nov 25, 2021
It is required to solve #6361
@github-actions

This comment has been minimized.

@Akuli
Copy link
Collaborator

Akuli commented Nov 28, 2021

ctypes.Array._length_ and ctypes.Array._type_ must be added to tests/stubtest_allowlists/py3_common.txt.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@Akuli Akuli merged commit d0ce310 into python:master Nov 28, 2021
@sobolevn
Copy link
Member Author

Thanks everyone!

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.

5 participants