Skip to content

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

Merged
ilevkivskyi merged 3 commits into
python:masterfrom
ilevkivskyi:another-fix-override
Sep 19, 2018
Merged

Don't crash on aliases like C = C#5632
ilevkivskyi merged 3 commits into
python:masterfrom
ilevkivskyi:another-fix-override

Conversation

@ilevkivskyi

@ilevkivskyi ilevkivskyi commented Sep 18, 2018

Copy link
Copy Markdown
Member

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.

@ilevkivskyi
ilevkivskyi requested a review from JukkaL September 18, 2018 13:22
@JelleZijlstra

JelleZijlstra commented Sep 18, 2018

Copy link
Copy Markdown
Member

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.

JelleZijlstra
JelleZijlstra reviewed Sep 18, 2018
View reviewed changes
Comment thread test-data/unit/check-incremental.test Outdated
import a
[file a.py]
import lib
x = 1

@JelleZijlstra JelleZijlstra Sep 18, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ilevkivskyi ilevkivskyi Sep 18, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@JelleZijlstra JelleZijlstra Sep 18, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ilevkivskyi

ilevkivskyi commented Sep 18, 2018

Copy link
Copy Markdown
Member Author

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.

@ilevkivskyi

ilevkivskyi commented Sep 19, 2018

Copy link
Copy Markdown
Member Author

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

@JukkaL

JukkaL commented Sep 19, 2018

Copy link
Copy Markdown
Collaborator

Looks good to me!

@JelleZijlstra

JelleZijlstra commented Sep 19, 2018

Copy link
Copy Markdown
Member

I'm happy with the additional tests.

@ilevkivskyi
ilevkivskyi merged commit 5e97a68 into python:master Sep 19, 2018
@ilevkivskyi
ilevkivskyi deleted the another-fix-override branch September 19, 2018 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments
@JukkaL JukkaL Awaiting requested review from JukkaL

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ilevkivskyi @JelleZijlstra @JukkaL