gh-93334: Fix homonym edge case in PathFinder.find_spec() #98100
Conversation
double backticks in news entry
jacobtylerwalls
requested review from
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
brettcannon
changed the title
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have made the requested changes; please review again (happy to continue adjusting the test setup to be more minimal, if there's a way) |
|
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
Lib/importlib/_bootstrap_external.py
Outdated
| module = sys.modules[parent_module_name] | ||
| except KeyError as e: | ||
| raise ModuleNotFoundError( | ||
| f"{parent_module_name} must be imported before finding {self._name}.", |
There was a problem hiding this comment.
| f"{parent_module_name} must be imported before finding {self._name}.", | |
| f"{parent_module_name!r} must be imported before finding {self._name!r}", |
Lib/importlib/_bootstrap_external.py
Outdated
| f"{parent_module_name} must be imported before finding {self._name}.", | ||
| name=parent_module_name, | ||
| ) from e | ||
| return getattr(module, path_attr_name) |
There was a problem hiding this comment.
| return getattr(module, path_attr_name) | |
| else: | |
| return getattr(module, path_attr_name) |
| msg = 'project4 must be imported before finding project4.foo.' | ||
| with self.assertRaisesRegex(ModuleNotFoundError, msg): | ||
| importlib.machinery.PathFinder.find_spec(submodule_path) |
There was a problem hiding this comment.
Being so specific on the exception message is very brittle, so we tend to avoid it. Instead, test any of the data attached to the exception as that's something that code might work with.
| msg = 'project4 must be imported before finding project4.foo.' | |
| with self.assertRaisesRegex(ModuleNotFoundError, msg): | |
| importlib.machinery.PathFinder.find_spec(submodule_path) | |
| with self.assertRaisesRegex(ModuleNotFoundError) as cm: | |
| importlib.machinery.PathFinder.find_spec(submodule_path) | |
| self.assertEqual(cm.excepion.name, 'project4') |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Thanks for the review! I have made the requested changes; please review again |
|
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
PathFinder.find_spec()can raise bareKeyErrorwhenpath=None#93334Closes #93334