Skip to content
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

Make collections.abc.* generic #5

Merged
merged 5 commits into from
Jan 31, 2020

Conversation

emmatyping
Copy link

@emmatyping emmatyping commented Jan 30, 2020

... and add tests for them.

I also removed __class_getitem__ from collections.UserDict/collections.UserList, since those inherit from collections.abc types (which is a nice test inheritance works for these types)!'

I didn't import types.GenericAlias in _collections_abc since this seems to be on the hot path for startup, and I figured that would be bad.

@gvanrossum
Copy link
Owner

Try running ./python.exe Lib/test/test_genericalias.py. There are three errors that occur without this too, but this one's new:


======================================================================
FAIL: test_collections_protocols_allowed (__main__.ProtocolTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/guido/pep585/Lib/test/test_typing.py", line 1381, in test_collections_protocols_allowed
    self.assertIsSubclass(B, Custom)
  File "/Users/guido/pep585/Lib/test/test_typing.py", line 40, in assertIsSubclass
    raise self.failureException(message)
AssertionError: <class '__main__.ProtocolTests.test_collections_protocols_allowed.<locals>.B'> is not a subclass of <class '__main__.ProtocolTests.test_collections_protocols_allowed.<locals>.Custom'>

@emmatyping
Copy link
Author

emmatyping commented Jan 30, 2020

The test failure was due to protocol runtime attribute compatibility checking. I disabled checking for __class_getitem__ because this would break existing code like in the test:

@runtime_checkable
class Custom(collections.abc.Iterable, Protocol):
        def close(self): ...

class B: # would now require an implementation of __class_getitem__
       def __iter__(self):
            return []
       def close(self):
            return 0

Copy link
Owner

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

You said that __class_getitem__ is inherited -- maybe you need fewer of them?

Lib/typing.py Outdated Show resolved Hide resolved
Lib/_collections_abc.py Outdated Show resolved Hide resolved
Lib/_collections_abc.py Outdated Show resolved Hide resolved
Lib/_collections_abc.py Outdated Show resolved Hide resolved
Lib/_collections_abc.py Outdated Show resolved Hide resolved
@emmatyping
Copy link
Author

You said that class_getitem is inherited -- maybe you need fewer of them?

Yep! Done.

Copy link
Owner

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Awesome! You left a few blank lines, in, but I'll take them.

@gvanrossum gvanrossum merged commit bb9a11f into gvanrossum:pep585 Jan 31, 2020
gvanrossum pushed a commit that referenced this pull request Jan 28, 2021
….1 (pythonGH-24289)

RFC 8018 superseded RFC 8018.

Automerge-Triggered-By: GH:tiran
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