-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Merge TypeVarDef and TypeVarType #9951
Conversation
Do we even need the separation between TypeVarDef and ParamSpecDef? There seems a lot of duplication. A single flag bit or maybe making ParamSpecDef a trivial subclass of the other might simplify things there. |
This comment has been minimized.
This comment has been minimized.
Yeah, it's conceivable it could be made a flag. But having it be its own type makes my development much easier, since it allows mypy to tell me where I need to add functionality. I already extracted what's common into |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
We should probably give people advanced notice about this change in #6617 since I expect several plugins deal with TypeVars. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a suggestion but ignore it if you don't agree.
And yeah, we should mention it in the plugin issue.
mypy/checkexpr.py
Outdated
@@ -3277,13 +3277,12 @@ def check_lst_expr(self, items: List[Expression], fullname: str, | |||
# Used for list and set expressions, as well as for tuples | |||
# containing star expressions that don't refer to a | |||
# Tuple. (Note: "lst" stands for list-set-tuple. :-) | |||
tvdef = TypeVarDef('T', 'T', -1, [], self.object_type()) | |||
tv = TypeVarType(tvdef) | |||
tvdef = TypeVarType('T', 'T', -1, [], self.object_type()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe keep the tv
names (and similar below) instead of the tvdef
ones?
And this is great. Honestly I never understood the distinction distinction and am happy to learn that this confusion was warranted. |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Diff from mypy_primer, showing the effect of this PR on open source code: pydantic (https://github.com/samuelcolvin/pydantic.git)
+ pydantic/mypy.py:41: error: Module "mypy.types" has no attribute "TypeVarDef"; maybe "TypeVar" or "TypeVarType"? [attr-defined]
+ pydantic/mypy.py:337: error: Missing positional arguments "fullname", "id", "values", "upper_bound" in call to "TypeVarType" [call-arg]
|
Problem: While using the Pydantic plugin with bleeding-edge Mypy I realized that Pydantic is using a recently-removed type. Solution: Find two instances of `TypeVarDef` and rename to `TypeVarType`. See-also: python/mypy#9951
Problem: While using the Pydantic plugin with bleeding-edge Mypy I realized that Pydantic is using a recently-removed type. Solution: Find two instances of `TypeVarDef` and rename to `TypeVarType`. See-also: python/mypy#9951
Problem: While using the Pydantic plugin with bleeding-edge Mypy I realized that Pydantic is using a recently-removed type. Solution: Find two instances of `TypeVarDef` and rename to `TypeVarType`. See-also: python/mypy#9951
In the spirit of #4814 (comment)
I'm not sure what the distinction between TypeVarDef and TypeVarType was (note that there's also TypeVarExpr, whose purpose is clear to me).
In practice, it seems it boiled down to: a) TypeVarDef has a different
__repr__
, that's not user facing but was used in tests, b) in two places, we used the construction of a TypeVarType from a TypeVarDef as an opportunity to change the line number.I ran into this because:
a) I was considering whether to create a corresponding ParamSpecDef and ParamSpecType
b) I was trying to fix an issue with applytype sometimes swallowing generics, which to fix I needed TypeVarType to have access to its TypeVarDef.
I was unable to find a reason for TypeVarType to not just be the TypeVarDef and here we are.