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

Fix inference when class and instance match protocol #18587

Merged
merged 3 commits into from
Feb 3, 2025

Conversation

ilevkivskyi
Copy link
Member

@ilevkivskyi ilevkivskyi commented Feb 2, 2025

Fixes #14688

The bug resulted from (accidentally) inferring against Iterable for both instance and class object. While working on this I noticed there are also couple flaws in direction handling in constrain inference, namely:

  • A protocol can never ever be a subtype of class object or a Type[X]
  • When matching against callback protocol, subtype check direction must match inference direction

I also (conservatively) fix some unrelated issues uncovered by the fix (to avoid fallout):

  • Callable subtyping with trivial suffixes was broken for positional-only args
  • Join of Parameters could lead to meaningless results in case of incompatible arg kinds
  • Protocol inference was inconsistent with protocol subtyping w.r.t. metaclasses.

This comment has been minimized.

@ilevkivskyi
Copy link
Member Author

OK, so the starlette thing used to work because of a chain of truly bizarre "coincidences" (bugs compensating bugs). It will take a while to unwind all this. This PR exposed a whole can of worms.

This comment has been minimized.

Copy link
Contributor

github-actions bot commented Feb 3, 2025

Diff from mypy_primer, showing the effect of this PR on open source code:

pandas (https://github.com/pandas-dev/pandas)
+ pandas/core/arrays/categorical.py:2939: error: Unused "type: ignore" comment  [unused-ignore]

django-stubs (https://github.com/typeddjango/django-stubs): 1.42x faster (55.6s -> 39.3s in a single noisy sample)

steam.py (https://github.com/Gobot1234/steam.py)
- steam/ext/commands/help.py:36: error: Argument 1 to "__init__" of "Command" has incompatible type "Callable[[Context[Any], DefaultNamedArg(str | None, 'content')], Coroutine[Any, Any, None]]"; expected "Callable[[None, Context[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, None]] | Callable[[Context[Any], VarArg(Any), KwArg(Any)], Coroutine[Any, Any, None]]"  [arg-type]
- steam/ext/commands/bot.py:203: error: Signature of "dispatch" incompatible with supertype "Client"  [override]
- steam/ext/commands/bot.py:203: note:      Superclass:
- steam/ext/commands/bot.py:203: note:          def dispatch(self, str, /, *args: Any, **kwargs: Any) -> None
- steam/ext/commands/bot.py:203: note:      Subclass:
- steam/ext/commands/bot.py:203: note:          def dispatch(self, event: str, *args: Any, **kwargs: Any) -> None

core (https://github.com/home-assistant/core)
+ homeassistant/components/sensor/const.py:593: error: Dict entry 39 has incompatible type "SensorDeviceClass": "set[UnitOfSpeed | UnitOfVolumetricFlux]"; expected "SensorDeviceClass": "set[type[StrEnum] | str | None]"  [dict-item]
+ homeassistant/components/number/const.py:497: error: Dict entry 39 has incompatible type "NumberDeviceClass": "set[UnitOfSpeed | UnitOfVolumetricFlux]"; expected "NumberDeviceClass": "set[type[StrEnum] | str | None]"  [dict-item]
+ homeassistant/helpers/entity_registry.py:90: error: Unused "type: ignore" comment  [unused-ignore]

@ilevkivskyi
Copy link
Member Author

OK, mypy_primer looks nice. This is now ready for review.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks, nice to see a fix to a highly voted issue! Looks good.

@JukkaL JukkaL merged commit 274af1c into python:master Feb 3, 2025
18 checks passed
@ilevkivskyi ilevkivskyi deleted the fix-str-enum branch February 3, 2025 15:03
@hauntsaninja
Copy link
Collaborator

These changes all make sense, thank you for fixing several chained bugs :-)

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.

Invalid typing assumption over variable with list(StrEnum) on python 3.11
3 participants