Skip to content

Make collections.abc.* generic #5

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.

Sign up for GitHub

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

Jump to bottom
Merged
gvanrossum merged 5 commits into from
Jan 31, 2020
Merged

Make collections.abc.* generic #5

gvanrossum merged 5 commits into from
Jan 31, 2020

Conversation

@emmatyping
Copy link

@emmatyping emmatyping commented Jan 30, 2020 •
edited
Loading

... 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

gvanrossum commented Jan 30, 2020

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 •
edited
Loading

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

@emmatyping emmatyping mentioned this pull request Jan 30, 2020
Merged
gvanrossum
gvanrossum reviewed Jan 30, 2020
View reviewed changes
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?

@emmatyping
Copy link
Author

emmatyping commented Jan 31, 2020

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

Yep! Done.

gvanrossum
gvanrossum approved these changes Jan 31, 2020
View reviewed changes
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
@illia-v
bpo-42996: Update a reference to PKCS #5 in hashlib docs to version 2…
b745a61 
….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

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@emmatyping @gvanrossum