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

modulefinder: make -p actually handle namespace packages correctly #9683

Merged
merged 6 commits into from
Nov 20, 2020

Conversation

hauntsaninja
Copy link
Collaborator

This is what #9616 should have been. I noticed this as part of #9672.

Note that this has the unfortunate side effect of causing mypy to fail when run on mypy with --namespace-packages, since typeshed is embedded inside mypy. This isn't great, especially in the context of #9636, and maybe indicates a missing "ignore" feature.

hauntsaninja added 6 commits October 31, 2020 15:34
This is important since the presence or absence of
self.options.namespace_packages should be visible
Something in Python 3.5 orders things differently. This is what I meant
anyway, so it's fine.
@JukkaL
Copy link
Collaborator

JukkaL commented Nov 20, 2020

Note that this has the unfortunate side effect of causing mypy to fail when run on mypy with --namespace-packages, since typeshed is embedded inside mypy.

FWIW, I'd like to have typeshed outside mypy, since being a subdirectory of mypy/ is often awkward.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

The code and test case update look fine, but I'm not sure what exactly is fixed by this PR. Based on the test case, this seems to related to __init__.py inside a namespace package? It would be good add more detail to the commit message or to create a GitHub issue about the bug that is fixed here.

@hauntsaninja
Copy link
Collaborator Author

hauntsaninja commented Nov 20, 2020

Yes, sorry about that! The core of the bugfix is the change from:

if os.path.isdir(abs_path) and \
        (os.path.isfile(os.path.join(abs_path, '__init__.py')) or
        os.path.isfile(os.path.join(abs_path, '__init__.pyi'))):

to

if self.fscache.isdir(path):
    if (self.options and self.options.namespace_packages) or (
        self.fscache.isfile(os.path.join(path, "__init__.py"))
        or self.fscache.isfile(os.path.join(path, "__init__.pyi"))
    ):

That is, once we were in a directory with __init__.py we only recursed into subdirectories if those subdirectories also contained an __init__.py.

@hauntsaninja hauntsaninja merged commit 40fd841 into python:master Nov 20, 2020
@hauntsaninja hauntsaninja deleted the 1031modfind2 branch November 20, 2020 18:26
hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this pull request Jan 18, 2021
This should cover the current state on master, as implemented
across python#9742, python#9683, python#9632, python#9616, python#9614, etc.

This will need to be changed if we can make `--namespace-packages` the
default (python#9636).
hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this pull request Jan 18, 2021
This should cover the current state on master, as previously
discussed / implemented across python#9742, python#9683, python#9632, python#9616, python#9614, etc.

This will need to be changed if we can make `--namespace-packages` the
default (python#9636).

I haven't documented some of the finer points of the changes, since it
felt like an inappropriate level of detail (e.g. using absolute paths
when crawling, how directories with invalid package names affect
crawling, etc)
hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this pull request Jan 18, 2021
This should cover the current state on master, as previously
discussed / implemented across python#9742, python#9683, python#9632, python#9616, python#9614, etc.

This will need to be changed if we can make `--namespace-packages` the
default (python#9636).

I haven't documented some of the finer points of the changes, since it
felt like an inappropriate level of detail (e.g. using absolute paths
when crawling, how directories with invalid package names affect
crawling, etc)
hauntsaninja added a commit that referenced this pull request Jan 19, 2021
This should cover the current state on master, as previously
discussed / implemented across #9742, #9683, #9632, #9616, #9614, etc.

This will need to be changed if we can make `--namespace-packages` the
default (#9636).

I haven't documented some of the finer points of the changes, since it
felt like an inappropriate level of detail (e.g. using absolute paths
when crawling, how directories with invalid package names affect
crawling, etc)

Co-authored-by: hauntsaninja <>
ilevkivskyi pushed a commit that referenced this pull request Jan 20, 2021
This should cover the current state on master, as previously
discussed / implemented across #9742, #9683, #9632, #9616, #9614, etc.

This will need to be changed if we can make `--namespace-packages` the
default (#9636).

I haven't documented some of the finer points of the changes, since it
felt like an inappropriate level of detail (e.g. using absolute paths
when crawling, how directories with invalid package names affect
crawling, etc)

Co-authored-by: hauntsaninja <>
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