Skip to content

Conversation

@jacobtylerwalls
Copy link
Member

@jacobtylerwalls jacobtylerwalls commented May 30, 2022

Steps

  • For new features or bug fixes, add a ChangeLog entry describing what your PR does.
  • Write a good description on what the PR does.

Description

Chaotic, I know, but I found additional KeyErrors, and when I compared the behavior after importing the packages, it suggested the original approach before #1575 was right. Any and all feedback welcome, I still find my footing uncertain.

For the tree a > b > c.py:

>>> from importlib.machinery import PathFinder
>>> PathFinder.find_spec('a.b', ['a'])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<frozen importlib._bootstrap_external>", line 1439, in find_spec
  File "<frozen importlib._bootstrap_external>", line 1218, in __init__
  File "<frozen importlib._bootstrap_external>", line 1233, in _get_parent_path
KeyError: 'a'
>>> import a.b.c  # now it's on sys.path
>>> a.b.c
<module 'a.b.c' from '/Users/jwalls/cpython/a/b/c.py'>
>>> PathFinder.find_spec('a')
ModuleSpec(name='a', loader=None, submodule_search_locations=_NamespacePath(['/Users/jwalls/cpython/a']))
>>> PathFinder.find_spec('a.b', path=['a'])
ModuleSpec(name='a.b', loader=None, submodule_search_locations=_NamespacePath(['/Users/jwalls/cpython/a/b']))
>>> PathFinder.find_spec('a.b.c', path=['a', 'a/b'])
ModuleSpec(name='a.b.c', loader=<_frozen_importlib_external.SourceFileLoader object at 0x104841420>, origin='/Users/jwalls/cpython/a/b/c.py')
origin='/Users/jwalls/cpython/a/b/c.py')
>>> PathFinder.find_spec('a.b.c', path=['a'])  # uh-oh, none! suggests #1575 was wrong

Type of Changes

Type
🐛 Bug fix

Related Issue

#1575

@coveralls
Copy link

coveralls commented May 30, 2022

Pull Request Test Coverage Report for Build 2411252360

  • 20 of 20 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 91.928%

Totals Coverage Status
Change from base Build 2410192088: 0.01%
Covered Lines: 9247
Relevant Lines: 10059

💛 - Coveralls

@Pierre-Sassoulas Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label May 30, 2022
@jacobtylerwalls jacobtylerwalls marked this pull request as ready for review May 30, 2022 18:06
@jacobtylerwalls jacobtylerwalls added this to the 2.12.0 milestone May 30, 2022
@jacobtylerwalls jacobtylerwalls requested review from DanielNoord and Pierre-Sassoulas and removed request for Pierre-Sassoulas May 30, 2022 18:07
@jacobtylerwalls
Copy link
Member Author

My hope is that the failure in keras goes away after merging this.

@jacobtylerwalls jacobtylerwalls merged commit 18483dc into pylint-dev:main May 30, 2022
@jacobtylerwalls jacobtylerwalls deleted the revert-1575 branch May 30, 2022 22:38
@jacobtylerwalls
Copy link
Member Author

Still a failure. But no sign of anything getting worse. I'll try to reproduce it -- maybe it was caused earlier than any of this.

@Pierre-Sassoulas
Copy link
Member

I saw python/mypy#11143 recently, mypy seems to have a nice system for import / sys.path issues, we could take some inspiration.

@jacobtylerwalls
Copy link
Member Author

@Pierre-Sassoulas @DanielNoord This appears to have fixed the OP's issue in pylint-dev/pylint#1361 ❗ 🤯 💥

@DanielNoord
Copy link
Collaborator

I mean, using pkg_resources to determine namespace packages was never going to be exhaustive 😄

Well done @jacobtylerwalls! Let's hope this resolves some more issues with namespace packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement ✨ Improvement to a component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants