Uh oh!
There was an error while loading. Please reload this page.
bpo-34632: Add importlib.metadata - #12547
Merged
Merged
Conversation
warsaw
reviewed
May 8, 2019
Uh oh!
There was an error while loading. Please reload this page.
warsaw
reviewed
May 8, 2019
Uh oh!
There was an error while loading. Please reload this page.
jaraco
commented
May 8, 2019
MemberAuthor
Looks now like failure is due to “test altered execution environment”. I think I saw |
jaraco
commented
May 10, 2019
MemberAuthor
I suspect here is where sys.path is modified, but it is configured for teardown... so I'm surprised that it's triggering an 'altered' state. |
jaraco
commented
May 10, 2019
MemberAuthor
Is the problem that |
MemberAuthor
It seems the cleanup isn't getting called at all. I see |
jaraco
commented
May 10, 2019
MemberAuthor
I'm able to replicate the "env changed" message locally: |
jaraco
commented
May 10, 2019
MemberAuthor
I can see that the cleanup is happening after the "env changed" test: |
MemberAuthor
I tried changing $ git diff diff --git a/Lib/test/test_importlib/fixtures.py b/Lib/test/test_importlib/fixtures.py
index 737ea4be7a..d0078d913d 100644
--- a/Lib/test/test_importlib/fixtures.py+++ b/Lib/test/test_importlib/fixtures.py@@ -28,6 +28,7 @@ def tempdir():
try:
yield pathlib.Path(tmpdir)
finally:
+ print('removing', tmpdir)
sys.path.remove(tmpdir)
shutil.rmtree(tmpdir)
@@ -59,9 +60,11 @@ class SiteDir:
def setUp(self):
self.fixtures = ExitStack()
- self.addCleanup(self.fixtures.close)
self.site_dir = self.fixtures.enter_context(self.site_dir())
+ def tearDown(self):+ self.fixtures.close()+
class DistInfoPkg(SiteDir):
files = {But in that case, the result is very similar: Details |
jaraco
commented
May 10, 2019
MemberAuthor
If I run the tests thus: $ git diff diff --git a/Lib/test/test_importlib/fixtures.py b/Lib/test/test_importlib/fixtures.py
index 737ea4be7a..bd5d0e48da 100644
--- a/Lib/test/test_importlib/fixtures.py+++ b/Lib/test/test_importlib/fixtures.py@@ -28,7 +28,9 @@ def tempdir():
try:
yield pathlib.Path(tmpdir)
finally:
+ print('removing', tmpdir)
sys.path.remove(tmpdir)
+ print(sys.path)
shutil.rmtree(tmpdir)I can see the |
jaraco
commented
May 10, 2019
MemberAuthor
Aha! So it seems that the path items are somehow getting added to |
jaraco
commented
May 10, 2019
MemberAuthor
In this ticket, I describe the issue and implemented the change, ported in 58533a7. |
jaraco
commented
May 11, 2019
MemberAuthor
@warsaw Tests are passing, so I think this is ready for your final review and merge. We can of course iterate over the coming weeks to refine the implementation. |
MemberAuthor
Oh - still needs:
|
Uh oh!
There was an error while loading. Please reload this page.
jaraco
commented
May 14, 2019
MemberAuthor
@warsaw I think we're ready to accept this. Is there anything more you want to see before landing it? |
warsaw
reviewed
May 24, 2019
| an iterator over instances of the ``Distribution`` abstract class. This | ||
| method must have the signature:: | ||
| def find_distributions(name=None, path=sys.path): |
Member
There was a problem hiding this comment.
We actually changed this signature to find_distributions(name=None, path=None) to account for changes in sys.path after the function's definition. See this MR.
warsaw
commented
May 24, 2019
Member
Hi @jaraco - I had just one comment about the documentation. If you can update that and resolve the conflict, I am ready to approve this PR. Thanks! |
jaraco
commented
May 24, 2019
MemberAuthor
Yes, good catch. I'm also planning to merge in the 0.14 release. I'll make sure this change happens with that as well. |
warsaw
commented
May 24, 2019
Member
I just tagged 0.15 with this fix (without updating the changelog :( ) but I'm not sure I did that correctly. |
jaraco
commented
May 24, 2019
MemberAuthor
OOh. Got almost all |
warsaw
approved these changes
May 24, 2019
warsaw
commented
May 24, 2019
Member
Sigh, if only GH had "merge when CI succeeds" button. :/ I will hover over "Squash and merge" as if I was trying to get an A position on Southwest. |
vstinner
commented
Jun 4, 2019
Member
This change should be mention in https://docs.python.org/dev/whatsnew/3.8.html : it's a new module, no? |
DinoV pushed a commit
to DinoV/cpython
that referenced
this pull request
Jan 14, 2020
Add importlib.metadata module as forward port of the standalone importlib_metadata.
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
This PR is a revised submission of #9327, based on bugfixes and improvements in importlib_metadata and taken directly from this branch.
https://bugs.python.org/issue34632