Use contravariant type variable rather than object in Container#15320
Use contravariant type variable rather than object in Container#15320srittau merged 5 commits intopython:mainfrom
object in Container#15320Conversation
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.
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]
|
|
Why did we need to add a default=Any? |
|
@hauntsaninja We didn't need to, I think. It was suggested in #15320 (comment), but I wasn't sure (#15320 (comment)). However, when in doubt I trust the maintainer's intuition. Would not using a default help with python/mypy#20729 (comment)? |
|
Thanks, I missed that exchange. My instinct is to avoid defaults when not needed, but Container is a bit of a weirdo. The default doesn't affect mypy's overlap check either way |
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)