Skip to content

MyPy v1.t6.0 cannot infer a descriptor's type through a Protocol which v1.15.0 supports #19274

Closed as duplicate

Description

@tdyas

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.

from dataclasses import dataclass
from typing import Any, 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]

@dataclass
class OptionInfo:
    flag_name: tuple[str, ...] | None


class _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,) if flag_name else None
        self._default = default
        return self

    def get_option_type(self, subsystem_cls):
        return type(self).option_type

    def _convert_(self, val: Any) -> _OptT:
        return cast("_OptT", val)

    def __set_name__(self, owner, name) -> None:
        if self._flag_names is None:
            kebab_name = name.strip("_").replace("_", "-")
            self._flag_names = (f"--{kebab_name}",)

    @overload
    def __get__(self, obj: None, objtype: Any) -> OptionInfo | None: ...

    @overload
    def __get__(self, obj: object, objtype: Any) -> _OptT: ...

    def __get__(self, obj, objtype):
        assert self._flag_names is not None
        if obj is None:
            return OptionInfo(self._flag_names)
        long_name = self._flag_names[-1]
        option_value = obj.options.get(long_name[2:].replace("-", "_"))
        if option_value is None:
            return None
        return self._convert_(option_value)


_IntDefault = TypeVar("_IntDefault", int, None)

class IntOption(_OptionBase[int, _IntDefault]):
    option_type: Any = int


class ExampleOption(IntOption):
    pass


class OptionConsumer:
    example = ExampleOption(default=None)

    @property
    def options(self):
        return {"example": 30}


class HasExampleOption(Protocol):
    example: ExampleOption


oc = OptionConsumer()
oc2 = cast(HasExampleOption, oc)

if TYPE_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
  • Python version used: 3.11.12

Activity

  1. sterliakov commented on Jun 11, 2025

    @sterliakov
    Collaborator

    Is it the same issue as below?

    from typing import Any, Protocol, overload
    
    class Descriptor:
        @overload
        def __get__(self, obj: None, objtype: Any) -> "Descriptor": ...
        @overload
        def __get__(self, obj: object, objtype: Any) -> int: ...
        def __get__(self, obj, objtype): ...
    
    class Foo:
        example = Descriptor()
    
    class FooP(Protocol):
        example: Descriptor
    
    
    oc = Foo()
    oc2: FooP = oc
    
    reveal_type(oc.example)
    reveal_type(oc2.example)

    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

  2. sterliakov commented on Jun 11, 2025

    @sterliakov
    Collaborator

    Whoops, sorry, you already answered. Closing as duplicate of #19054 (sorry, I somehow missed that one entirely...)

  3. ilevkivskyi commented on Jun 11, 2025

    @ilevkivskyi
    Member

    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

    class P(Protocol):
        attr: SomeType

    is equivalent to this in a regular class:

    class C:
        def __init__(self) -> None:
            self.attr: SomeType

    Now (on current master) you should be able to write simply this:

    class HasExampleOption(Protocol):
        example: int

    However, now I am thinking we should probably also allow something like this:

    class HasExampleOption(Protocol):
        example: ClassVar[ExampleOption]

    Let me play a bit with this.

  4. sterliakov commented on Jun 11, 2025

    @sterliakov
    Collaborator

    @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).

  5. ilevkivskyi commented on Jun 11, 2025

    @ilevkivskyi
    Member

    @sterliakov

    Assignability is more worrying

    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).

  6. sterliakov commented on Jun 11, 2025

    @sterliakov
    Collaborator

    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.

  7. ilevkivskyi commented on Jun 11, 2025

    @ilevkivskyi
    Member

    @sterliakov

    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:

    • Class variables (explicit and implicit)
    • Instance variables (explicit and implicit)
    • Instance variable with default
    • Descriptors
    • Variables with callable types
    • Type[...]
    • And probably other edge cases I forgot
  8. ilevkivskyi commented on Jun 11, 2025

    @ilevkivskyi
    Member

    Yeah, I will probably make a PR soon.

    For now I think we can do something like #19277

  9. tdyas commented on Jun 11, 2025

    @tdyas
    Author

    Thanks for the explanations!

    Some data points for any future work here:

    • 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.

  10. sterliakov commented on Jun 11, 2025

    @sterliakov
    Collaborator

    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.

  11. tdyas commented on Jun 11, 2025

    @tdyas
    Author

    Tested with git+https://github.com/python/mypy@61cbe0c3cada14eb3638de8becff8988ae8cb1db and it passes.

  12. JukkaL commented on Jun 11, 2025

    @JukkaL
    Collaborator

    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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugmypy got something wrongtopic-descriptorsProperties, class vs. instance attributes

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions