FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Avoid narrowing to NewType by hauntsaninja · Pull Request #20766 · python/mypy · GitHub

Repository navigation

Avoid narrowing to NewType - #20766

Merged
hauntsaninja merged 1 commit into
python:masterfrom
hauntsaninja:narrow90
Feb 14, 2026
Merged

hauntsaninja merged 1 commit into
python:masterfrom
hauntsaninja:narrow90

Conversation

hauntsaninja commented Feb 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Fixes #20733

I might make this type munging look a little nicer in a future refactor

This introduces new errors in two primer projects

github-actions Bot commented Feb 8, 2026

Copy link
Copy Markdown
Contributor

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

steam.py (https://github.com/Gobot1234/steam.py)
+ steam/ext/csgo/state.py:213: error: Argument 1 to "__delitem__" of "dict" has incompatible type "int"; expected "AssetID"  [arg-type]
+ steam/ext/csgo/state.py:214: error: Argument 1 to "__delitem__" of "dict" has incompatible type "int"; expected "AssetID"  [arg-type]

rotki (https://github.com/rotki/rotki)
- rotkehlchen/chain/evm/decoding/zerox/decoder.py:146: error: Unused "type: ignore" comment  [unused-ignore]
- rotkehlchen/chain/ethereum/modules/sky/decoder.py:147: error: Unused "type: ignore" comment  [unused-ignore]

Copy link
Copy Markdown
Collaborator Author

rotki was fixed in #20492 and is regressed here
steam was fixed in #20602 and is regressed here

JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Are you planning to address the regressions in this PR, or should we merge this as is, since this fixes a more important regression?


def f2(whatever: object, uid: UserId | Other):
if whatever == uid:
reveal_type(whatever) # N: Revealed type is "builtins.int | __main__.Other"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is a bit questionable, since whatever could be float or another numeric type that happens to compare equal to int (not directly related to the changes in this PR).

hauntsaninja commented Feb 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

The regressions are from what the PR does (no longer narrowing to a NewType), so nothing to address beyond deciding what behaviour we want mypy to have.

(I don't have a strong opinion / am not a big NewType user myself. Allowing narrowing is good on primer and the implementation is simpler and consistent with subclassing, but I can see the argument made in #20733 about having to be explicit)

JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I like that NewTypes require an explicit constructor call.

hauntsaninja changed the title Avoid narrowing NewType Avoid narrowing to NewType Feb 12, 2026
hauntsaninja merged commit a2d715a into python:master Feb 14, 2026
23 checks passed
hauntsaninja deleted the narrow90 branch February 14, 2026 01:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

Regression: Type of literal narrowed when asserting equality to a NewType

2 participants


Back | FazBrowse Home | New Git URL