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
Properly handle PEP 420 namespace packages #10183
Merged
Eric-Arellano
merged 10 commits into
pantsbuild:master
from
Eric-Arellano:stop-generating-inits
Jun 29, 2020
Merged
Properly handle PEP 420 namespace packages #10183
Eric-Arellano
merged 10 commits into
pantsbuild:master
from
Eric-Arellano:stop-generating-inits
Jun 29, 2020
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
benjyw
approved these changes
Jun 27, 2020
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.
Yay! Much better.
I can't figure out how to get PEP 420 working with MyPy # Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
Why is MyPy so much harder to get working?! # Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
…it__.py # Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
…ting-inits [ci skip-rust-tests]
…ting-inits [ci skip-rust-tests]
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
# Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
MyPy is still not working like I'd expect, but going to merge this because it makes substantial progress for multiple different Python tasks. Will fix MyPy in a dedicated PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When using PEP 420 namespace packages, you must leave off the
__init__.py
file. However, Pants was automatically injecting__init__.py
files when missing, which would break PEP 420.For example, Toolchain does not use
__init__.py
and has bothsrc/python/toolchain/
andtest/python/toolchain/
, which both map to the moduletoolchain
. So, v2 MyPy was failing because it detected an empty__init__.py
file in both packages.Solution
Stop ever generating
__init__.py
files, now that we automatically read them from the filesystem via #10166.This means that Pants will much closer emulate real Python semantics. If a user is using PEP 420, we'll respect that. If they are missing
__init__.py
and it would cause normal Python to fail, we will also fail, rather than trying to magically fix the issue.This PR also fixes our auto-discovery of
PathGlobs
when the input snapshot was stripped. The logic failed to account for how the input is stripped, yetPathGlobs
is never stripped.[ci skip-rust-tests]