Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Provide first component of dotted path to namespace searches #1575

Merged
merged 8 commits into from
May 30, 2022
Merged

Provide first component of dotted path to namespace searches #1575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3a49500
Use first component of dotted path only in namespace searches
jacobtylerwalls May 29, 2022
2897be0
Add test and catch KeyError instead of altering search strategy
jacobtylerwalls May 29, 2022
f795317
"module" -> "directory"
jacobtylerwalls May 30, 2022
6191382
reduce repetition
jacobtylerwalls May 30, 2022
1fd9729
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] May 30, 2022
1fb111a
typo
jacobtylerwalls May 30, 2022
2c3b781
add TODO
jacobtylerwalls May 30, 2022
02a4514
Search under the highest package name
jacobtylerwalls May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions astroid/interpreter/_import/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@ def is_namespace(modname: str) -> bool:
# That's unacceptable here, so we fallback to _find_spec_from_path(), which does
# not, but requires instead that each single parent ('astroid', 'nodes', etc.)
# be specced from left to right.
processed_components = []
last_parent = None
for component in modname.split("."):
processed_components.append(component)
working_modname = ".".join(processed_components)
components = modname.split(".")
for i in range(1, len(components) + 1):
working_modname = ".".join(components[:i])
try:
found_spec = _find_spec_from_path(working_modname, last_parent)
# Search under the highest package name
# Only relevant if package not already on sys.path
# See https:/python/cpython/issues/89754 for reasoning
# Otherwise can raise bare KeyError: https:/python/cpython/issues/93334
found_spec = _find_spec_from_path(working_modname, components[0])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jacobtylerwalls Just thinking about this again. We cached this function because of importlib being slow in #1536 (comment). (My very first review comment there).

What if we try to get the spec of:
a.b.c.d.e.f.g and then a.b.c.d.e.f.h? We don't benefit from any of the caching right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but I don't know if we ever confirmed that this use of importlib was slow. It's slow if importlib is actually importing the parent modules (why we did the #1409 to revert) but the whole point of this approach was to not do that, so I'm not sure it's an issue.

except ValueError:
# executed .pth files may not have __spec__
return True
last_parent = working_modname

if found_spec is None:
return False
Expand Down
Empty file added tests/testdata/python3/data/parent_of_homonym/__init__.py
View file
Open in desktop
Empty file.
2 changes: 2 additions & 0 deletions tests/testdata/python3/data/parent_of_homonym/doc/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
This submodule should have the same name as a directory in the root of the project that
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point we should really move these files to a tests/data directory instead of this one...

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas May 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is definitely some cleanup to do in the tests directory, there is also the issue of legacy functional tests. https:/PyCQA/pylint/blob/main/tests/test_func.py#L117 nvm it's in pylint

is _NOT_ a python module. For this reason, it's currently called "doc".
Empty file added tests/testdata/python3/data/parent_of_homonym/doc/__init__.py
View file
Open in desktop
Empty file.
5 changes: 5 additions & 0 deletions tests/unittest_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ def test_identify_old_namespace_package_protocol(self) -> None:
util.is_namespace("tests.testdata.python3.data.path_pkg_resources_1")
)

def test_submodule_homonym_with_non_module(self) -> None:
self.assertFalse(
util.is_namespace("tests.testdata.python3.data.parent_of_homonym.doc")
)

def test_implicit_namespace_package(self) -> None:
data_dir = os.path.dirname(resources.find("data/namespace_pep_420"))
contribute = os.path.join(data_dir, "contribute_to_namespace")
Expand Down