-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Use contravariant type variable rather than object in Container
#15320
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
Conversation
This comment has been minimized.
This comment has been minimized.
object in Containerobject in Container
|
cc @hauntsaninja @AlexWaygood as you authored #8785, what do you think of this change? |
|
The only notable primer change is from def filter_tasks(self, tasks: Container[Task]) -> Iterable[tuple[str, Task | None]]:
for item in self.record:
if item[0] in ("schedule", "before", "after") and item[1] in tasks:
yield itemsince |
This comment has been minimized.
This comment has been minimized.
AlexWaygood
left a comment
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 makes sense to me, thanks
|
Diff from mypy_primer, showing the effect of this PR on open source code: mypy (https://github.com/python/mypy)
+ mypy/server/aststrip.py:140: error: Unused "type: ignore" comment [unused-ignore]
trio (https://github.com/python-trio/trio)
+ src/trio/_core/_tests/test_instrumentation.py:40: error: Unsupported operand types for in ("Task | None" and "Container[Task]") [operator]
dulwich (https://github.com/dulwich/dulwich)
+ dulwich/refs.py:127: error: Unused "type: ignore" comment [unused-ignore]
+ dulwich/refs.py:129: error: Unused "type: ignore" comment [unused-ignore]
+ dulwich/refs.py:131: error: Unused "type: ignore" comment [unused-ignore]
+ dulwich/refs.py:140: error: Unused "type: ignore" comment [unused-ignore]
+ dulwich/refs.py:142: error: Unused "type: ignore" comment [unused-ignore]
urllib3 (https://github.com/urllib3/urllib3)
+ test/test_collections.py:336: error: Unused "type: ignore" comment [unused-ignore]
+ test/test_collections.py:337: error: Unused "type: ignore" comment [unused-ignore]
+ test/test_collections.py:398: error: Unused "type: ignore" comment [unused-ignore]
+ test/test_collections.py:399: error: Unused "type: ignore" comment [unused-ignore]
core (https://github.com/home-assistant/core)
+ homeassistant/components/homekit/__init__.py:981: error: Unused "type: ignore" comment [unused-ignore]
ibis (https://github.com/ibis-project/ibis)
- ibis/expr/datatypes/core.py:613: error: Argument 1 of "__contains__" is incompatible with supertype "typing.Container"; supertype defines the argument type as "object" [override]
discord.py (https://github.com/Rapptz/discord.py)
- ...typeshed_to_test/stdlib/typing.pyi:1046: note: "update" of "TypedDict" defined here
+ ...typeshed_to_test/stdlib/typing.pyi:1049: note: "update" of "TypedDict" defined here
operator (https://github.com/canonical/operator)
- ops/model.py:885: error: Argument 1 of "__contains__" is incompatible with supertype "typing.Container"; supertype defines the argument type as "object" [override]
- ops/model.py:932: error: Argument 1 of "__contains__" is incompatible with supertype "typing.Container"; supertype defines the argument type as "object" [override]
- ops/model.py:1961: error: Argument 1 of "__contains__" is incompatible with supertype "typing.Container"; supertype defines the argument type as "object" [override]
- ops/model.py:2402: error: Argument 1 of "__contains__" is incompatible with supertype "typing.Container"; supertype defines the argument type as "object" [override]
|
Fixes #15307
See Also: #15319
This PR changes the generic type of
collections.abc.Containerfromcovarianttocontravariantand lets thekeybe of this type rather thanobject.The current definition forces a lot ofpeople to resort to
type: ignore[override].Potential Backward Compatibility issue
The way I modified the class, if someone uses something like
This change will make their class invariant, where it was covariant before.
Inside
collections.abc.CollectionI addressed this issue by changingContainer[_T_co]toContainer[Any].There weren't any other cases in the whole stubs where a type variable was fed into
Container, so I believe the breakage in real world code will be extremely limited (as the user also would have have to use PEP 695 style syntax)