Skip to content
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

mypy claims compatible types are incompatible when non-related mixin classes are present #12440

Open
zzzeek opened this issue Mar 24, 2022 · 9 comments
Labels
bug mypy got something wrong topic-inheritance Inheritance and incompatible overrides

Comments

@zzzeek
Copy link

zzzeek commented Mar 24, 2022

as always, I apologize if this falls into one of the categories of "that's expected, because what if you did this?" and/or "this is a known mypy limitation / issue number", this one does not have a great workaround since I'd have to "type:ignore" the class def itself, or have to add redundant attributes everywhere, and I will likely just re-organize the code to not have this particular issue.

mypy --version
mypy 0.950+dev.0fecec8c72605ec7bae343b28d545c477c726e81 (compiled: no)

Here's the demo - ImplementsWOMixin and ImplementsWMixin are basically the same class except one of them has an extra entry in its __bases__. mypy then decides that a local attribute that returns str instead of Optional[str] is no longer compatible with the base class:

from typing import Optional


class Base:
    description: Optional[str]


class MixinWDesc:
    description: str


class SubClass(MixinWDesc, Base):
    description: str


class SomeOtherMixin:
    pass

# succeeds
class ImplementsWOMixin(SubClass):
    pass


# fails
class ImplementsWMixin(SomeOtherMixin, SubClass):
    pass

output - it complains about MixinWDesc, but the line number is that of the ImplementsWMixin class itself, meaning every class that subclasses MixinWDesc and uses other mixins would need workarounds or type:ignores.

$ mypy test3.py 
test3.py:26: error: Definition of "description" in base class "MixinWDesc" is incompatible with definition in base class "Base"  [misc]
Found 1 error in 1 file (checked 1 source file)
@zzzeek zzzeek added the bug mypy got something wrong label Mar 24, 2022
@zzzeek
Copy link
Author

zzzeek commented Mar 24, 2022

Easier workaround is I will change MixinWDesc alone:

class MixinWDesc:
    description: Optional[str]

@AlexWaygood AlexWaygood added the topic-inheritance Inheritance and incompatible overrides label Mar 24, 2022
@JelleZijlstra
Copy link
Member

This error is correct. Your type annotations define a read-write attribute, so a function that gets an object b: Base is allowed to set b.description = None. But MixinWDesc requires that it's not None.

To prevent this error, make the attribute read-only by using a property.

@zzzeek
Copy link
Author

zzzeek commented Mar 24, 2022

why does ImplementsWMixin and ImplementsWOMixin behave differently?

@zzzeek
Copy link
Author

zzzeek commented Mar 24, 2022

also I generally cant use @property due to #4125. hopefully mypy can recognize other descriptor types that don't have __set__, will find out

@zzzeek
Copy link
Author

zzzeek commented Mar 24, 2022

nope , it doesn't work:

from typing import Optional, TypeVar, Generic, Any, Callable, overload, Union

_T_co = TypeVar("_T_co", covariant=True)



_GFDRO = TypeVar("_GFDRO", bound="generic_ro_descriptor[Any]")


class generic_ro_descriptor(Generic[_T_co]):
    fget: Callable[..., _T_co]
    __doc__: Optional[str]
    __name__: str

    def __init__(self, fget: Callable[..., _T_co], doc: Optional[str] = None):
        self.fget = fget  # type: ignore[assignment]
        self.__doc__ = doc or fget.__doc__
        self.__name__ = fget.__name__

    @overload
    def __get__(self: _GFDRO, obj: None, cls: Any) -> _GFDRO:
        ...

    @overload
    def __get__(self, obj: object, cls: Any) -> _T_co:
        ...

    def __get__(self: _GFDRO, obj: Any, cls: Any) -> Union[_GFDRO, _T_co]:
        raise NotImplementedError()

    # whether or not these are here
    def __set__(self, instance: Any, value: Any) -> NoReturn:
        ...

    def __delete__(self, instance: Any) -> NoReturn:
        ...



class Base:
    @generic_ro_descriptor
    def description(self) -> Optional[str]:
        ...


class MixinWDesc:
    @generic_ro_descriptor
    def description(self) -> str:
        ...


class SubClass(MixinWDesc, Base):
    @generic_ro_descriptor
    def description(self) -> str:
        ...


class SomeOtherMixin:
    pass

# succeeds
class ImplementsWOMixin(SubClass):
    pass


# fails:
#  Definition of "description" in base class "MixinWDesc" is incompatible with definition in base class "Base"
class ImplementsWMixin(SomeOtherMixin, SubClass):
    pass

is @property the only way Mypy sees something as "read only" ? a descriptor with no __set__ method, or __set__ method that returns NoReturn is not sufficient ?

@zzzeek
Copy link
Author

zzzeek commented Mar 24, 2022

OK, I guess I will try to use the "tell typing it's @property but use your own descriptor" trick...

@JelleZijlstra
Copy link
Member

Good point, it should complain about Subclass but not the other ones.

@JelleZijlstra JelleZijlstra reopened this Mar 24, 2022
@zzzeek
Copy link
Author

zzzeek commented May 1, 2022

still wondering if there's any way to make my own descriptor that is not @property that mypy sees as "read only". I would think a descriptor class that has __get__() but not __set__(), or perhaps a __set__() that returns NoReturn, would be seen as a read-only property, but at the moment I think this is not the case (going to test a little more today). (edit: I guess I already did all this above and none of it works. Some clear docs on this whole area would be helpful).

I still encounter inconvenient behaviors with @property that are difficult to debug because I dont know what's hardcoded with @property and is beyond the scope of making sense without reading mypy's source code.

@zzzeek
Copy link
Author

zzzeek commented May 1, 2022

or otherwise, if the whole "mypy has a special rule that @Property can be used to make a read only field" could be better documented, the only mention I can find for this concept is as a passing phrase here, "You can use @property to make an attribute read-only, but unlike Final, it doesn’t work with module attributes, and it doesn’t prevent overriding in subclasses." that whole concept ("you can use @property, and @property alone, not any other kind of descriptor, to make an attribute read-only") is not obvious to someone who doesnt know it already. the variety of hardcoded "property" behaviors should really be documented, there should be a section in mypy's docs with all the special rules that are present right now listed out ( as well as which are exclusive to @property alone and not user-defined descriptors).

I would gladly contribute this documentation if I myself knew the behaviors well, but I only have a vague concept right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong topic-inheritance Inheritance and incompatible overrides
Projects
None yet
Development

No branches or pull requests

3 participants