Skip to content

Fix #2308: Keep suppressed imports in the suppressed list #3065

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

Merged
merged 2 commits into from
Mar 28, 2017

Conversation

refi64
Copy link
Contributor

@refi64 refi64 commented Mar 27, 2017

The problem here was a logic discrepancy.

In one side of the code, suppressed modules need to be inside manager.modules_missing in order to stay suppressed. In the other side of the code, suppressed modules are never checked, meaning they are never added to that run's modules_missing.

There's a 99% chance this fix is mostly wrong, because the incremental code is really freaking complicated, but at least I get an A for effort, right? ;)

@gvanrossum
Copy link
Member

This needs a test that fails without the fix.

@refi64
Copy link
Contributor Author

refi64 commented Mar 28, 2017

@gvanrossum Only problem is that the only failing test case I've been able to reproduce requires three incremental runs, but the tests only make room for two...

@gvanrossum
Copy link
Member

Oh, I've had a few of those. We may have to design a more general mechanism (Jukka is already inventing those in #2838.) Can you at least include instructions to reproduce the issue here so I can see for myself?

@refi64
Copy link
Contributor Author

refi64 commented Mar 28, 2017

#2308 (comment):

Create the following files:

  • x.py:

    A = 0
    from y import B
  • y.py:

    import xyz  # type: ignore
    B = 0
    from x import A

Now, watch the following commands:

ryan@DevPC-LX ~/blaze-mypy/bug master $ mypy -i x.py  # Works.
ryan@DevPC-LX ~/blaze-mypy/bug master $ touch x.py    # Update the mtime of x.py.
ryan@DevPC-LX ~/blaze-mypy/bug master $ mypy -i x.py  # Works.
ryan@DevPC-LX ~/blaze-mypy/bug master $ touch x.py    # Update the mtime of x.py again.
ryan@DevPC-LX ~/blaze-mypy/bug master $ mypy -i x.py  # Fails!
x.py:2: note: In module imported here:
y.py:1: error: Cannot find module named 'xyz'
y.py:1: note: (Perhaps setting MYPYPATH or using the "--silent-imports" flag would help)
ryan@DevPC-LX ~/blaze-mypy/bug master $ 

@gvanrossum
Copy link
Member

Heh, you have an ancient version of mypy installed, but I can still repro this on master, so thanks!

@refi64
Copy link
Contributor Author

refi64 commented Mar 28, 2017

@gvanrossum Well, that was a copy-paste of the error I had gotten several months ago... :)

Swap the if/elif branches so each condition gets tested only once.
@gvanrossum
Copy link
Member

LGTM. I tweaked the logic a bit. Will merge when the tests pass. Thanks!

@gvanrossum gvanrossum merged commit 76824c9 into python:master Mar 28, 2017
@gvanrossum
Copy link
Member

Thanks! I somehow missed the reference to the original issue, hence my insistence on a test case. Sorry for that. These bugs are nasty and the fixes are rarely obvious, so thanks for your help!

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

Successfully merging this pull request may close these issues.

2 participants