MyPy v1.t6.0 is no longer able to infer a descriptor's type through a Protocol which it could do in v1.15.0.
To Reproduce
The following code is a simplified form of the Pantsbuild option system in this file.
fromdataclassesimportdataclassfromtypingimportAny, Protocol, Union, TypeVar, Callable, Generic, cast, TYPE_CHECKING, overload# The type of the option._OptT=TypeVar("_OptT")
# The type of option's default (may be _OptT or some other type like `None`)_DefaultT=TypeVar("_DefaultT")
_SubsystemType=Any_DynamicDefaultT=Callable[[_SubsystemType], Any]
_MaybeDynamicT=Union[_DynamicDefaultT, _DefaultT]
@dataclassclassOptionInfo:
flag_name: tuple[str, ...] |Noneclass_OptionBase(Generic[_OptT, _DefaultT]):
_flag_names: tuple[str, ...] |None_default: _MaybeDynamicT[_DefaultT]
def__new__(
cls,
flag_name: str|None=None,
*,
default: _MaybeDynamicT[_DefaultT]
):
self=super().__new__(cls)
self._flag_names= (flag_name,) ifflag_nameelseNoneself._default=defaultreturnselfdefget_option_type(self, subsystem_cls):
returntype(self).option_typedef_convert_(self, val: Any) ->_OptT:
returncast("_OptT", val)
def__set_name__(self, owner, name) ->None:
ifself._flag_namesisNone:
kebab_name=name.strip("_").replace("_", "-")
self._flag_names= (f"--{kebab_name}",)
@overloaddef__get__(self, obj: None, objtype: Any) ->OptionInfo|None: ...
@overloaddef__get__(self, obj: object, objtype: Any) ->_OptT: ...
def__get__(self, obj, objtype):
assertself._flag_namesisnotNoneifobjisNone:
returnOptionInfo(self._flag_names)
long_name=self._flag_names[-1]
option_value=obj.options.get(long_name[2:].replace("-", "_"))
ifoption_valueisNone:
returnNonereturnself._convert_(option_value)
_IntDefault=TypeVar("_IntDefault", int, None)
classIntOption(_OptionBase[int, _IntDefault]):
option_type: Any=intclassExampleOption(IntOption):
passclassOptionConsumer:
example=ExampleOption(default=None)
@propertydefoptions(self):
return {"example": 30}
classHasExampleOption(Protocol):
example: ExampleOptionoc=OptionConsumer()
oc2=cast(HasExampleOption, oc)
ifTYPE_CHECKING:
reveal_type(oc.example)
reveal_type(oc2.example)
(This code could probably be further simplified, but is sufficient to reproduce the bug. )
Expected Behavior
The expected behavior is that the access through the HasExampleOption protocol to the example attribute has type int. Here is the output when running v1.15.0 against the code:
src/grok.py:90: note: Revealed type is "builtins.int"
src/grok.py:91: note: Revealed type is "builtins.int"
Actual Behavior
v1.16.0 regresses and reports the type of example as the descriptor's class ExampleOption and not as int:
src/grok.py:90: note: Revealed type is "builtins.int"
src/grok.py:91: note: Revealed type is "grok.ExampleOption"
Your Environment
Mypy version used: v1.15.0 and v1.16.0
Mypy command-line flags: none
Mypy configuration options from mypy.ini (and other config files): none
I'm not exactly sure what should protocol members annotated with descriptors mean, TBH. mypy is now inconsistent (note that assignability above is fine). However, x: Something in protocol means "instance.x is compatible with Something", and that's not the case here, so revealing descriptor and rejecting assignment also sounds reasonable to me.
Yes, this is an intentional change. For long time mypy didn't support descriptors in protocols, so unfortunately some people used a workaround of writing descriptor type directly in the protocol. The motivation for the change is that protocols only specify the interface, so that we can reason about e.g. modules and class objects as implementation of protocols. By default, something like this
@ilevkivskyi ClassVar[Descriptor] sounds interesting - somewhat closer to runtime semantics. And I agree using descriptor itself in annotation is too ambiguous to argue about, the new approach is more reasonable IMO (because, again, simple protocols roughly mean "this instance has an attribute of this type", and instances don't have descriptor-valued attributes). Assignability is more worrying: member access and assignability rules should stay in sync (oc2: FooP = oc should not be allowed too).
I am not sure what do you mean? On current master I get an error in oc2: FooP = oc. Or do you mean in v1.16? If latter, then yeah, it was a mistake to have a release in between the two protocol vs descriptor PRs. This may be confusing for users. As an option (and what I suggested initially to @JukkaL) we may revert the first PR in the release branch (and include the revert in a point release).
Ough, I definitely need another coffee. Yes, works as intended on master, it's only 1.16 that is broken. Perhaps consider releasing 1.16.1 (either with some PRs cherry-picked or just from current master if there aren't any huge breaking changes)?
Aside, I still think your ClassVar suggestion would be nice to implement. That would support some runtime introspection better and allow almost-consistent implementation and interface definition.
Aside, I still think your ClassVar suggestion would be nice to implement.
Yeah, I will probably make a PR soon.
Btw another thing is that last few days I was looking into the whole class variable vs instance variable story, and it is really messy. (And this is because in Python the distinction is blurry, similar to how we have enormous amount of complexity because positional-or-keyword arguments.) Maybe I would go as far as suggesting this is a topic for the typing council discussion. We should have some clear rules for:
I tried the suggestion of switching the example attribute's type to int from the ExampleOption descriptor. That worked when the only type being considered was HasExampleOption.
That switch though caused a new type error. The owner class of the descriptor mixes in HasExampleOption and now has a type error similar to this:
src/python/pants/core/goals/fix.py:411:9: error: Argument 5 to "_do_fix" has incompatible type "FixSubsystem"; expected "_BatchableMultiToolGoalSubsystem" [arg-type]
fix_subsystem,
^~~~~~~~~~~~~
src/python/pants/core/goals/fix.py:411:9: note: Following member(s) of "FixSubsystem" have conflicts:
src/python/pants/core/goals/fix.py:411:9: note: batch_size: expected "int", got "BatchSizeOption"
(_BatchableMultiToolGoalSubsystem is the concrete example of what is HasExampleOption in the reproduction code in this PR. And BatchSizeOption is the equivalent of ExampleOption.)
Thus, the owner class is no longer compliant with the protocol when switching the type of the descriptor in the protocol like that.
Could you check the same using master mypy (pip install 'git+https://github.com/python/mypy')? That will be slower, but the diagnostic is very similar to the discrepancy we discussed above, it should pass on master.
i will make a 1.16.1 bugfix release soon, but I think it won't include a fix to this issue. However, we are planning to start work on the 1.17 feature release pretty soon (hopefully within a week, but we'll see).
Activity
sterliakov commented on Jun 11, 2025
Is it the same issue as below?
https://mypy-play.net/?mypy=latest&python=3.12&flags=strict&gist=62b88bc128305716903821b1b3333f29
I'm not exactly sure what should protocol members annotated with descriptors mean, TBH. mypy is now inconsistent (note that assignability above is fine). However, x: Something in protocol means "instance.x is compatible with Something", and that's not the case here, so revealing descriptor and rejecting assignment also sounds reasonable to me.
@ilevkivskyi was this change intentional? Bisects to #18831
sterliakov commented on Jun 11, 2025
Whoops, sorry, you already answered. Closing as duplicate of #19054 (sorry, I somehow missed that one entirely...)
ilevkivskyi commented on Jun 11, 2025
Yes, this is an intentional change. For long time mypy didn't support descriptors in protocols, so unfortunately some people used a workaround of writing descriptor type directly in the protocol. The motivation for the change is that protocols only specify the interface, so that we can reason about e.g. modules and class objects as implementation of protocols. By default, something like this
is equivalent to this in a regular class:
Now (on current master) you should be able to write simply this:
However, now I am thinking we should probably also allow something like this:
Let me play a bit with this.
sterliakov commented on Jun 11, 2025
@ilevkivskyi ClassVar[Descriptor] sounds interesting - somewhat closer to runtime semantics. And I agree using descriptor itself in annotation is too ambiguous to argue about, the new approach is more reasonable IMO (because, again, simple protocols roughly mean "this instance has an attribute of this type", and instances don't have descriptor-valued attributes). Assignability is more worrying: member access and assignability rules should stay in sync (oc2: FooP = oc should not be allowed too).
ilevkivskyi commented on Jun 11, 2025
@sterliakov
I am not sure what do you mean? On current master I get an error in oc2: FooP = oc. Or do you mean in v1.16? If latter, then yeah, it was a mistake to have a release in between the two protocol vs descriptor PRs. This may be confusing for users. As an option (and what I suggested initially to @JukkaL) we may revert the first PR in the release branch (and include the revert in a point release).
sterliakov commented on Jun 11, 2025
Ough, I definitely need another coffee. Yes, works as intended on master, it's only 1.16 that is broken. Perhaps consider releasing 1.16.1 (either with some PRs cherry-picked or just from current master if there aren't any huge breaking changes)?
Aside, I still think your ClassVar suggestion would be nice to implement. That would support some runtime introspection better and allow almost-consistent implementation and interface definition.
ilevkivskyi commented on Jun 11, 2025
@sterliakov
Yeah, I will probably make a PR soon.
Btw another thing is that last few days I was looking into the whole class variable vs instance variable story, and it is really messy. (And this is because in Python the distinction is blurry, similar to how we have enormous amount of complexity because positional-or-keyword arguments.) Maybe I would go as far as suggesting this is a topic for the typing council discussion. We should have some clear rules for:
ilevkivskyi commented on Jun 11, 2025
For now I think we can do something like #19277
tdyas commented on Jun 11, 2025
Thanks for the explanations!
Some data points for any future work here:
src/python/pants/core/goals/fix.py:411:9: error: Argument 5 to "_do_fix" has incompatible type "FixSubsystem"; expected "_BatchableMultiToolGoalSubsystem" [arg-type] fix_subsystem, ^~~~~~~~~~~~~ src/python/pants/core/goals/fix.py:411:9: note: Following member(s) of "FixSubsystem" have conflicts: src/python/pants/core/goals/fix.py:411:9: note: batch_size: expected "int", got "BatchSizeOption"(_BatchableMultiToolGoalSubsystem is the concrete example of what is HasExampleOption in the reproduction code in this PR. And BatchSizeOption is the equivalent of ExampleOption.)
Thus, the owner class is no longer compliant with the protocol when switching the type of the descriptor in the protocol like that.
sterliakov commented on Jun 11, 2025
Could you check the same using master mypy (pip install 'git+https://github.com/python/mypy')? That will be slower, but the diagnostic is very similar to the discrepancy we discussed above, it should pass on master.
tdyas commented on Jun 11, 2025
Tested with git+https://github.com/python/mypy@61cbe0c3cada14eb3638de8becff8988ae8cb1db and it passes.
JukkaL commented on Jun 11, 2025
i will make a 1.16.1 bugfix release soon, but I think it won't include a fix to this issue. However, we are planning to start work on the 1.17 feature release pretty soon (hopefully within a week, but we'll see).
Handle corner case: protocol vs classvar vs descriptor (#19277)
Handle corner case: protocol vs classvar vs descriptor (#19277)