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

Don't crash on aliases like C = C #5632

Merged
merged 3 commits into from Sep 19, 2018

Conversation

Projects
None yet
3 participants
@ilevkivskyi
Collaborator

ilevkivskyi commented Sep 18, 2018

Fixes #5631

Unfortunately one still needs to use # type: ignores, mypy has troubles understanding complex conditional definitions like in the issue:

try:
    ConnectionError = ConnectionError
except NameError:
    class ConnectionError(Exception):
        pass

But this is a separate problem, that is harder to solve.

Ivan Levkivskyi

@ilevkivskyi ilevkivskyi requested a review from JukkaL Sep 18, 2018

@JelleZijlstra

This comment has been minimized.

Collaborator

JelleZijlstra commented Sep 18, 2018

Could you add a test that ensures that if module a has Exception = Exception, it is possible to use a.Exception and it's equivalent to builtins.Exception? I think that's what the pip code from the original issue was doing.

Ivan Levkivskyi
import a
[file a.py]
import lib
x = 1

This comment has been minimized.

@JelleZijlstra

JelleZijlstra Sep 18, 2018

Collaborator

Thanks! Can you also check that using lib.Exception works here?

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Sep 18, 2018

Collaborator

Do you mean in this file? I can add try/except here.

This comment has been minimized.

@JelleZijlstra

JelleZijlstra Sep 18, 2018

Collaborator

Yes, in this file. Something like try: ... except lib.Exception:.

@ilevkivskyi

This comment has been minimized.

Collaborator

ilevkivskyi commented Sep 18, 2018

and it's equivalent to builtins.Exception

I am not sure what you mean by this, but even just int = int will confuse mypy. This is unrelated to this particular crash/fix, but is due to the fact that when resolving r.h.s. mypy will just find the Var defined by l.h.s. This is harder to fix, as I mentioned, but it was always like this.

Ivan Levkivskyi
@ilevkivskyi

This comment has been minimized.

Collaborator

ilevkivskyi commented Sep 19, 2018

@JelleZijlstra @JukkaL what is the situation here? Are there more comments?

@JukkaL

This comment has been minimized.

Collaborator

JukkaL commented Sep 19, 2018

Looks good to me!

@JelleZijlstra

This comment has been minimized.

Collaborator

JelleZijlstra commented Sep 19, 2018

I'm happy with the additional tests.

@ilevkivskyi ilevkivskyi merged commit 5e97a68 into python:master Sep 19, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ilevkivskyi ilevkivskyi deleted the ilevkivskyi:another-fix-override branch Sep 19, 2018

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