-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Namespace packages (PEP 420) #5691
Conversation
f2c87ff
to
ecac1b5
Compare
(Whoops, accidentally deleted the branch which caused the PR to be closed. I'm not done yet.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is right. At least after reading the PEP and playing a bit, this coincides with the runtime behaviour. I have two more suggestions for tests (apart from nesting normal in namespace and vice versa as you proposed, I just checked, both of these work at runtime).
As I understand this fixes #1645. I would add it to the PR description, so that we will not forget to close the issue :) |
Alas, tests for nesting one type of package inside the other fail (both ways -- it always prefers the decoy file). It's late so I'll just leave it broken, I'll fix it tomorrow. |
Hey @carljm, IIRC you expressed interest in namespace packages eons ago. Perhaps you can test this PR with your use case? |
(Hold on, I have an idea for a refinement of the final part of the algorithm.) |
@ilevkivskyi I think I'm happy now. Are you? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am happy with this.
mypy/modulefinder.py
Outdated
# indicates the highest level at which a __init__.py[i] file | ||
# is found; if no __init__ was found it returns 0, if we find | ||
# only foo/bar/__init__.py it returns 1, and if we have | ||
# foo/__init__.py it returns 2 (regardless of what's un |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: 'un' -> 'in'
Tentative implementation of PEP 420. Fixes python#1645. Clarification of the implementation: - `candidate_base_dirs` is a list of `(dir, verify)` tuples, laboriously pre-computed. - The old code just looped over this list, looking for `dir/<module>`, and if found, it would verify that there were `__init__.py` files in all the right places (except if `verify` is false); the first success would be the hit; - In PEP 420 mode, if the above approach finds no hits, we do something different for those paths that failed due to `__init__` verification; essentially we narrow down the list of candidate paths by checking for `__init__` files from the top down. Hopefully the last test added clarifies this.
Tentative implementation of PEP 420. Fixes #1645.
Clarification of the implementation:
candidate_base_dirs
is a list of(dir, verify)
tuples, laboriously pre-computed.dir/<module>
, and if found, it would verify that there were__init__.py
files in all the right places (except ifverify
is false); the first success would be the hit;__init__
verification; essentially we narrow down the list of candidate paths by checking for__init__
files from the top down. Hopefully the last test added clarifies this.