You signed in with another tab or window.Reload to refresh your session.You signed out in another tab or window.Reload to refresh your session.You switched accounts on another tab or window.Reload to refresh your session.Dismiss alert
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
The reason will be displayed to describe this comment to others.Learn more.
Types should benever modified in place. I know you didn't introduce it, but this should be fixed ASAP. (I think there is also a chance some of the problems you try to fix may be caused by this).
The reason will be displayed to describe this comment to others.Learn more.
Types should be never modified in place.
So, withcopy_modified then? If stuff should never be modified in place, it surprises me that the variables are not annotated asFinal in the first place.
The reason will be displayed to describe this comment to others.Learn more.
Hm, If I change this then now 2 tests fail:testTypeVarDefaultsClassRecursive1 andtestTypeVarDefaultsClassRecursiveMultipleFiles
Details
____________________________________________________________________________________________________________________________________ testTypeVarDefaultsClassRecursive1 _____________________________________________________________________________________________________________________________________[gw0] linux -- Python 3.13.8 /home/rscholz/github/CONTRIBUTING/mypy/.venv/bin/python3data: /home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test:451:Failed: Unexpected type checker output (/home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test, line 451)------------------------------------------------------------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------------------------------------------------------------Expected: main:16: note: Revealed type is "__main__.ClassD1[builtins.str, builtins.str]" main:17: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]" main:18: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]" main:21: note: Revealed type is "__main__.ClassD1[builtins.str, builtins.str]" (diff) main:23: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]" main:25: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]" main:35: note: Revealed type is "__main__.ClassD2[builtins.str, builtins.str, builtins.str]" main:36: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, builtins.int]" main:37: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]" main:38: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]" main:41: note: Revealed type is "__main__.ClassD2[builtins.str, builtins.str, builtins.str]" (diff) main:43: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, builtins.int]" (diff) main:45: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]" main:47: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]" main:56: note: Revealed type is "__main__.ClassD3[builtins.str, builtins.list[builtins.str]]" ... main:58: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]" main:63: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.list[builtins.int]]" main:65: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]"Actual: main:16: note: Revealed type is "__main__.ClassD1[builtins.str, builtins.str]" main:17: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]" main:18: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]" main:21: note: Revealed type is "__main__.ClassD1[builtins.str, T1`1 = builtins.str]" (diff) main:23: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.int]" main:25: note: Revealed type is "__main__.ClassD1[builtins.int, builtins.float]" main:35: note: Revealed type is "__main__.ClassD2[builtins.str, builtins.str, builtins.str]" main:36: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, builtins.int]" main:37: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]" main:38: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]" main:41: note: Revealed type is "__main__.ClassD2[builtins.str, T1`1 = builtins.str, T2`2 = T1`1 = builtins.str]" (diff) main:43: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.int, T2`2 = builtins.int]" (diff) main:45: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.float]" main:47: note: Revealed type is "__main__.ClassD2[builtins.int, builtins.float, builtins.str]" main:56: note: Revealed type is "__main__.ClassD3[builtins.str, builtins.list[builtins.str]]" ... main:58: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]" main:63: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.list[builtins.int]]" main:65: note: Revealed type is "__main__.ClassD3[builtins.int, builtins.float]"Alignment of first line difference: E: ..._.ClassD1[builtins.str, builtins.str]" A: ..._.ClassD1[builtins.str, T1`1 = builtins.str]" ^Update the test output using --update-data (implies -n0; you can additionally use the -k selector to update only specific tests)______________________________________________________________________________________________________________________________ testTypeVarDefaultsClassRecursiveMultipleFiles _______________________________________________________________________________________________________________________________[gw0] linux -- Python 3.13.8 /home/rscholz/github/CONTRIBUTING/mypy/.venv/bin/python3data: /home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test:518:Failed: Unexpected type checker output (/home/rscholz/github/CONTRIBUTING/mypy/test-data/unit/check-typevar-defaults.test, line 518)------------------------------------------------------------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------------------------------------------------------------Expected: main:15: note: Revealed type is "__main__.ClassG1[builtins.int, builtins.int]" main:16: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]" main:17: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]" main:20: note: Revealed type is "__main__.ClassG1[builtins.int, builtins.int]" (diff) main:22: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]" main:24: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]"Actual: main:15: note: Revealed type is "__main__.ClassG1[builtins.int, builtins.int]" main:16: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]" main:17: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]" main:20: note: Revealed type is "__main__.ClassG1[builtins.int, T2`1 = builtins.int]" (diff) main:22: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.str]" main:24: note: Revealed type is "__main__.ClassG1[builtins.str, builtins.float]"Alignment of first line difference: E: ..._.ClassG1[builtins.int, builtins.int]" A: ..._.ClassG1[builtins.int, T2`1 = builtins.int]" ^Update the test output using --update-data (implies -n0; you can additionally use the -k selector to update only specific tests)========================================================================================================================================== short test summary info ==========================================================================================================================================FAILED mypy/test/testcheck.py::TypeCheckSuite::check-typevar-defaults.test::testTypeVarDefaultsClassRecursive1FAILED mypy/test/testcheck.py::TypeCheckSuite::check-typevar-defaults.test::testTypeVarDefaultsClassRecursiveMultipleFiles
I noticed thatTypeVarType.__hash__ is given byhash((self.id, self.upper_bound, tuple(self.values))), so changingdefault does not change the hash.
The reason will be displayed to describe this comment to others.Learn more.
Yes. For historical reasons some types are constructed "gradually", so we can't make type attributes final. Maybe we will at some point, but that would be a big refactoring. For now, mypy contributors should remember a simple rule: types always new, symbols always original.
The reason will be displayed to describe this comment to others.Learn more.
If I change this then now 2 tests fail
Oh well, it means there is another bug. TBH I am not sure if default should be part of the hash conceptually (and what it even means to have a type variable with same id, but a different default). IMO the support for type variable defaults may need to be re-implemented from scratch, as it is problematic on a conceptual level: regular default is not a property of an argument, it is a property of a function; same for type variables, their defaults are not property of type variable type itself, but of the function/class that uses them.
Modifyingrepl.default in-place ensures that in theExpandTypeVisitor.variables dictionary, the default of the dependent type variable gets updated to the new ID.
The reason will be displayed to describe this comment to others.Learn more.
TBH I don't understand why do we need to handle the defaults inexpand_type in the first place. As I explained, the default is not a property of a type variable, it is a property of a class/function, so the caller (e.g.applytype etc) should always provide acomplete type var -> replacement mapping. So thatexpand_type() should not be even allowed to look at the defaults.
Yeah, I just looked at the original PR that added this logic#16878, it looks broken. The full complete mapping should be computed either in:
semanal/typeanal for "static" application likex: C[int]
inapplytype for "dynamic" application likex = C[int]()
before we even callexpand_type. I think the only right way to do it is this way.
The reason will be displayed to describe this comment to others.Learn more.
I am not sure how to fix this at the moment, but given that this PR
Fixes two very annoying bugs with PEP696 checking (essentially prevents typing bijections with defaults)
Doesn't introduce inplace modification -- it was already there before (all we really do is make sure to update the default a-priori, which is crucial e.g. when the second typevar defaults to the first one and we expand the first one.)
This issue probably needs to be addressed elsewhere
The reason will be displayed to describe this comment to others.Learn more.
The downside is that this is papering over the problem instead of fixing the real root cause. But anyway, if you feel strongly about this, I can merge this, it will not be much more work to axe this all later (and the tests are useful).
expanded = repl.accept(self) # Note: `expanded is repl` may be true.
repl = repl if isinstance(expanded, TypeVarType) else expanded
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
reveal_type(m) # N: Revealed type is "__main__.ClassC4[builtins.int, builtins.float]"
[builtins fixtures/tuple.pyi]
[case testTypeVarDefaultsSwap]
from typing import TypeVar, Generic
T = TypeVar("T")
X = TypeVar("X", default=object)
Y = TypeVar("Y", default=object)
class Foo(Generic[T, Y]):
def test(self) -> None:
reveal_type( Foo[Y, T]() ) # N: Revealed type is "__main__.Foo[Y`2 = builtins.object, T`1]"
class Bar(Generic[X, Y]):
def test(self) -> None:
reveal_type( Bar[Y, X]() ) # N: Revealed type is "__main__.Bar[Y`2 = builtins.object, X`1 = builtins.object]"
[case testTypeVarDefaultsSwap2]
from typing import TypeVar, Generic
X = TypeVar("X", default=object)
Y = TypeVar("Y", default=object)
U = TypeVar("U", default=object)
V = TypeVar("V", default=object)
class Transform(Generic[X, Y]):
def invert(self) -> "Transform[Y, X]": ...
class Foo(Transform[U, V], Generic[U, V]):
def invert(self) -> "Foo[V, U]": ...
[case testTypeVarDefaultsClassRecursive1]
# flags: --disallow-any-generics
from typing import Generic, TypeVar, List
Expand Down
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.