Avoid narrowing to NewType#20766
Conversation
hauntsaninja
force-pushed
the
narrow90
branch
from
fb282a9 to
2842819
Compare
hauntsaninja
force-pushed
the
narrow90
branch
from
2842819 to
fc3ccde
Compare
|
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]
|
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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).
|
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
JukkaL
left a comment
There was a problem hiding this comment.
I like that NewTypes require an explicit constructor call.
2 participants
Fixes #20733
I might make this type munging look a little nicer in a future refactor
This introduces new errors in two primer projects