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

Using mypy with mixins and abstract properties #7631

Closed
Xyene opened this issue Oct 4, 2019 · 2 comments · Fixed by #7713
Closed

Using mypy with mixins and abstract properties #7631

Xyene opened this issue Oct 4, 2019 · 2 comments · Fixed by #7713
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal

Comments

@Xyene
Copy link
Contributor

Xyene commented Oct 4, 2019

  • Are you reporting a bug, or opening a feature request?

Unclear, I'm assuming it's not a bug. It's possible (very likely) that this is a duplicate issue, but I haven't been able to find the likely dupe.

  • Please insert below the code you are checking with mypy,
    or a mock-up repro if the source is private. We would appreciate
    if you try to simplify your case to a minimal repro.
from abc import *


class SomeABC(metaclass=ABCMeta):
    @property
    @abstractmethod
    def foo(cls) -> str:
        pass


class SomeMixin:
    foo = "foo"


class SomeClass(SomeMixin, SomeABC):
    pass
  • What is the actual behavior/output?
a.py:15: error: Definition of "foo" in base class "SomeMixin" is incompatible with definition in base class "SomeABC"

Using a mixin to satisfy abstract properties doesn't seem like too uncommon of a use-case, so I'm erring on the side of me doing something wrong, but I haven't been able to get the code above to type-check.

  • What is the behavior/output you expect?

I would hope to see no errors while typechecking. Currently a workaround is to make the property not-abstract, but that trades runtime safety for mypy checking on that file.

  • What are the versions of mypy and Python you are using?
$ mypy --version
mypy 0.730
$ python3 --version
Python 3.7.3
  • Do you see the same issue after installing mypy from Git master?
    Yes, with mypy 0.740+dev.cc7667a2008ef4cdcf89b06ab4e171bf478b0734.

  • What are the mypy flags you are using? (For example --strict-optional)
    None.

Thanks!

@Xyene Xyene changed the title Using mypy with mixins and abstract classes Using mypy with mixins and abstract properties Oct 4, 2019
@Xyene
Copy link
Contributor Author

Xyene commented Oct 5, 2019

This poorly-formatted and probably-broken PoC patch resolves the issue I'm seeing without breaking any tests. I'm happy to clean up the code if support for this should exist in mypy.

From a4404591d131eae119c4ad55042bc356af770592 Mon Sep 17 00:00:00 2001
From: Tudor Brindus <me@tbrindus.ca>
Date: Fri, 4 Oct 2019 20:10:40 -0400
Subject: [PATCH] Hack for abstract properties and mixins

---
 mypy/checker.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mypy/checker.py b/mypy/checker.py
index bdfda80f..27db5111 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -1855,6 +1855,10 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                 ok = is_subtype(first_sig, second_sig, ignore_pos_arg_names=True)
         elif first_type and second_type:
             ok = is_equivalent(first_type, second_type)
+            if not ok and name in base2.abstract_attributes:
+                second_node = base2[name].node
+                if isinstance(second_node, Decorator) and second_node.func.is_property:
+                    ok = is_subtype(first_type, cast(CallableType, second_type).ret_type)
         else:
             if first_type is None:
                 self.msg.cannot_determine_type_in_base(name, base1.name(), ctx)
-- 
2.20.1

@JukkaL JukkaL added bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal labels Oct 7, 2019
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 7, 2019

Yeah, this looks like a mypy bug. Your suggested fix is along the right lines -- there may be some edge cases to consider but we'd be happy to accept a PR and help you clean it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-1-normal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants