Skip to content

Add --no-implicit-reexport flag #6562

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 6 commits into from
Jun 4, 2019

Conversation

jhance
Copy link
Collaborator

@jhance jhance commented Mar 18, 2019

Adds a --no-implicit-reexport flag, intended for giving modules the same behavior as stubs for re-exporting their imports.

The goal is to make it possible to prevent bad transitive imports from causing dependency issues. And to make it easier to move things to a different module.

@jhance
Copy link
Collaborator Author

jhance commented Mar 18, 2019

There may be poor behavior if this is enabled without --no-ignore-missing-imports as it will just turn all the stuff to Any

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

  • Remove no from the option name and revert the default (but keep --no prefix in flag)
  • Add docs (for both flag and option)
  • Update the comments (they only refer to stubs)
  • Can you come up with an example of the poor interaction when ignore_missing_imports is not enabled?

@msullivan
Copy link
Collaborator

I've thought about it some and the interactions with ignore_missing_imports seems fine? Ignore missing imports ignores missing modules, not missing attributes.

I'm going to try to get mypy to check clean with this option and report back on that.

msullivan added a commit that referenced this pull request Mar 19, 2019
This makes mypy check clean with the proposed --no-implicit-reexports
flag from PR #6562.

Some of the implicit reexports that were depended on were clearly
intended (type_visitor) and so made explicit, while some were clearly
unintended (Optional from mypy.types!) and fixed.

Probably worth landing this whichever way we decide on #6562, but I
think this PR is an argument in favor.
@msullivan
Copy link
Collaborator

#6570 fixes mypy to check clean with this.

The error output was intimidating at first (because of all the knock-on effects), but when I just looked at the import failures it was very simple to tackle.

(It may be a good idea to treat imports of non-public names differently: produce an error on the import, but still resolve the name to the private symbol, to reduce the error volume and to provide better type checking if the import error gets ignored. That's neither here nor there for this bug.)

msullivan added a commit that referenced this pull request Mar 20, 2019
This makes mypy check clean with the proposed --no-implicit-reexports
flag from PR #6562.

Some of the implicit reexports that were depended on were clearly
intended (type_visitor) and so made explicit, while some were clearly
unintended (Optional from mypy.types!) and fixed.

Probably worth landing this whichever way we decide on #6562, but I
think this PR is an argument in favor.
@jhance jhance force-pushed the no-implicit-reexport branch from bafecfe to d61833f Compare June 4, 2019 16:33
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

LG (assuming tests now pass).

[case testNoImplicitReexport]
# flags: --no-implicit-reexport
from other_module_2 import a

Copy link
Member

Choose a reason for hiding this comment

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

I'd remove all the blank lines from the test cases except the blank line between test cases (so no blank lines between files within a test case).

# This won't re-export the value
from foo import bar
# This will re-export it as bar and allow other modules to import it
from foo import bar as bar
Copy link
Member

Choose a reason for hiding this comment

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

Indentation is still wrong here.

@ilevkivskyi ilevkivskyi merged commit 7cc438d into python:master Jun 4, 2019
PattenR pushed a commit to PattenR/mypy that referenced this pull request Jun 23, 2019
Adds a --no-implicit-reexport flag, intended for giving modules the same behavior as stubs for re-exporting their imports.

The goal is to make it possible to prevent bad transitive imports from causing dependency issues. And to make it easier to move things to a different module.
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.

4 participants