Skip to content

[wip] Better handling of generics when narrowing#20673

Draft
hauntsaninja wants to merge 1 commit intopython:masterfrom
hauntsaninja:narrow54
Draft

[wip] Better handling of generics when narrowing#20673
hauntsaninja wants to merge 1 commit intopython:masterfrom
hauntsaninja:narrow54

Conversation

@hauntsaninja
Copy link
Collaborator

@hauntsaninja hauntsaninja commented Jan 28, 2026

Notably we preserve behaviour on the testNarrowingCollections test I added

mypy/checker.py Outdated
# See testMatchEnumSingleChoice
target_type = coerce_to_literal(target_type)
if isinstance(target_type, Instance):
target_type = erase_type(target_type)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We might want to distribute this over unions, but we do want to avoid erasure for several different types. Let's see if there is primer...

@hauntsaninja hauntsaninja force-pushed the narrow54 branch 2 times, most recently from 67e6322 to 1e598b3 Compare January 28, 2026 21:43
@github-actions

This comment has been minimized.

@hauntsaninja hauntsaninja marked this pull request as draft January 28, 2026 22:33
@hauntsaninja hauntsaninja changed the title Better handling of generics when narrowing [wip] Better handling of generics when narrowing Jan 28, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

JukkaL pushed a commit that referenced this pull request Feb 2, 2026
Fixes #20701 / see discussion there

Background:
- In #20492 I added code to treat
bytes, bytearray and memoryview as having custom `__eq__`, since they
can compare equal to values that are of other types. I did this based on
primer hits on draft versions of that PR
- In #20643 I rewrote the handling of
custom equality when narrowing. That fixed many issues, but regressed
the equivalent of `testNarrowingOptionalBytes` because we're now more
principled about custom `__eq__`. Perhaps interestingly the primer on
that PR is small and not controversial, so I didn't feel the need to
prioritise unsound things

However, we only need one of the operands to have custom equality to
model these things as reachable, so I think we can probably just remove
`builtins.bytes` from here.

This behaviour still isn't ideal. For instance, narrowing here shouldn't
depend on whether we promote or not (we shouldn't narrow `v_all` to
bytes)

The real fix is adding more dedicated special casing so that we have an
in-between option between "narrow assuming we can only compare equal to
values of the same type" and "assume `__eq__` means we can't conclude
anything about the positive case"

Other relevant PRs:
- #20673 attempts to remove the other
custom eq logic for list, dict, set I added in #20492 . It should be
close to mergeable
- #20660 improves reachability and
unfortunately to land it I have to do a rethink for how int, float,
complex narrowing works. I actually think the current version has pretty
good semantics and the primer is valid (but scary). But I want to think
about it more
@hauntsaninja hauntsaninja force-pushed the narrow54 branch 2 times, most recently from 4fb55a5 to ffd48e0 Compare February 6, 2026 22:33
@github-actions

This comment has been minimized.

@hauntsaninja hauntsaninja force-pushed the narrow54 branch 2 times, most recently from 0c48b00 to c417de8 Compare February 6, 2026 23:28
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2026

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

graphql-core (https://github.com/graphql-python/graphql-core)
- tests/execution/test_resolve.py:322: error: Value of type "list[SourceLocation] | None" is not indexable  [index]

core (https://github.com/home-assistant/core)
+ homeassistant/components/philips_js/media_player.py:203: error: Argument 1 to "get_browse_image_url" of "MediaPlayerEntity" has incompatible type "str | None"; expected "str"  [arg-type]
+ homeassistant/components/isy994/sensor.py:206: error: Argument 2 to "temperature" of "UnitSystem" has incompatible type "str | None"; expected "str"  [arg-type]

rich (https://github.com/Textualize/rich)
+ rich/console.py:1540: error: Redundant cast to "Literal['left', 'center', 'right']"  [redundant-cast]

discord.py (https://github.com/Rapptz/discord.py)
- discord/channel.py:3048: error: Argument "stickers" to "handle_message_parameters" has incompatible type "Sequence[GuildSticker | StickerItem]"; expected "list[str | int] | None"  [arg-type]
+ discord/channel.py:3048: error: Argument "stickers" to "handle_message_parameters" has incompatible type "Sequence[Any]"; expected "list[str | int] | None"  [arg-type]
- discord/ext/tasks/__init__.py:765: error: Incompatible types in assignment (expression has type "time | Sequence[time]", variable has type "list[time]")  [assignment]
+ discord/ext/tasks/__init__.py:765: error: Incompatible types in assignment (expression has type "time | Sequence[Any]", variable has type "list[time]")  [assignment]

dulwich (https://github.com/dulwich/dulwich)
+ dulwich/lru_cache.py:164: error: Item "None" of "_LRUNode[K, V] | None" has no attribute "prev"  [union-attr]
+ dulwich/lru_cache.py:171: error: Item "None" of "_LRUNode[K, V] | None" has no attribute "prev"  [union-attr]
+ dulwich/lru_cache.py:171: error: Item "None" of "_LRUNode[K, V] | None" has no attribute "key"  [union-attr]
+ dulwich/lru_cache.py:175: error: Incompatible types in "yield" (actual type "_LRUNode[K, V] | None", expected type "_LRUNode[K, V]")  [misc]

rotki (https://github.com/rotki/rotki)
+ rotkehlchen/tests/integration/test_premium.py:820: error: Statement is unreachable  [unreachable]
+ rotkehlchen/tests/integration/test_premium.py:834: error: Statement is unreachable  [unreachable]

xarray (https://github.com/pydata/xarray)
+ xarray/tests/test_dataarray.py:291: error: Unused "type: ignore" comment  [unused-ignore]

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.

1 participant