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

Subclasses of Twisted's protocol fail to be determined by mypy-zope #34

Closed
clokep opened this issue Mar 10, 2021 · 8 comments · Fixed by #41 or twisted/twisted#1565
Closed

Subclasses of Twisted's protocol fail to be determined by mypy-zope #34

clokep opened this issue Mar 10, 2021 · 8 comments · Fixed by #41 or twisted/twisted#1565

Comments

@clokep
Copy link
Contributor

clokep commented Mar 10, 2021

Thanks for making mypy-zope! We've been using it for Synapse (which uses Twisted) and ran into a hiccup.

The following will cause mypy-zope to emit an error:

error: Cannot determine consistent method resolution order (MRO) for "Foo" [misc]

from twisted.internet import protocol

class Foo(protocol.Protocol):
    pass

reveal_type(Foo)

The class hierarchy looks like Foo -> Protocol (which implements IProtocol and ILoggingContext) -> BaseProtocol.

I tried to create a reduced test-case for this, but was not able to, so it might be specific to something Twisted has done. Any help would be appreciated!

@clokep clokep changed the title T Subclasses of Twisted's protocol fail to be determined by mypy-zope Mar 10, 2021
@kedder
Copy link
Member

kedder commented Mar 11, 2021

@clokep two questions:

  1. Do you get this error when running mypy with cold caches (i.e. first time after you delete .mypy_cache directory?
  2. Do you get this error if you are running mypy daemon? (i.e. by running dmypy run)

@clokep
Copy link
Contributor Author

clokep commented Mar 11, 2021

Do you get this error when running mypy with cold caches (i.e. first time after you delete .mypy_cache directory?

Nope!

$ rm -r .mypy_cache
$ mypy temp.py
temp.py:6: note: Revealed type is 'def () -> temp.Foo'
$ mypy temp.py
temp.py:3: error: Cannot determine consistent method resolution order (MRO) for "Foo"  [misc]
temp.py:6: note: Revealed type is 'def () -> temp.Foo'
Found 1 error in 1 file (checked 1 source file)

Do you get this error if you are running mypy daemon? (i.e. by running dmypy run)

I don't think so, but I've never used this before so I'm not 100% sure I'm using it properly.

@clokep
Copy link
Contributor Author

clokep commented Mar 11, 2021

I should also mention the versions of mypy I'm using:

mypy==0.812
mypy-extensions==0.4.3
mypy-zope==0.2.11

This is with the latest Twisted (Twisted==21.2.0).

@kedder
Copy link
Member

kedder commented Mar 11, 2021

Ok, then I think I'm observing the same problem in my project. We mainly use dmypy so this doesn't annoy us very much and we learned to ignore it. Maybe it is time to figure it out.

@clokep
Copy link
Contributor Author

clokep commented Mar 11, 2021

Ok, then I think I'm observing the same problem in my project. We mainly use dmypy so this doesn't annoy us very much and we learned to ignore it.

I had completely forgotten dmypy existed until you mentioned it!

Maybe it is time to figure it out.

I'd appreciate it. Please let me know if I can help at all! 😄

@clokep
Copy link
Contributor Author

clokep commented Mar 22, 2021

Any idea where one would start poking at to try to fix this? Does mypy-zope do something in particular to hook into the caching mechanism?

@kedder
Copy link
Member

kedder commented Mar 22, 2021

This might be a bit tricky. I think the reason we are getting this error is because of this hack: we are adding interface class to the MRO of all implementation classes in order for this assignment to work:

foo: IFoo = Foo()

mypy will consider IFoo as a supertype of Foo and therefore permit the assignment.

But at the same time, I believe having this interface in the MRO list confuses up the MRO calculation in some cases (makes the inheritance graph unsortable, or smth like that).

I'm not yet sure how to address that, but perhaps we need to find another way to support foo: IFoo = Foo() type of assignments.

@clokep
Copy link
Contributor Author

clokep commented Mar 22, 2021

This is hopefully fixed by #41, that at least fixes the example given in the description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants