From 08d64b3873bf1001456244c1feaa302d7e504f26 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 12:40:40 -0400 Subject: [PATCH 1/7] Revert #1575 and catch further `KeyError`s --- astroid/interpreter/_import/util.py | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py index 47623c94d8..e14bd4ea6a 100644 --- a/astroid/interpreter/_import/util.py +++ b/astroid/interpreter/_import/util.py @@ -18,18 +18,28 @@ 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. - components = modname.split(".") - for i in range(1, len(components) + 1): - working_modname = ".".join(components[:i]) + processed_components = [] + last_parent = None + for component in modname.split("."): + processed_components.append(component) + working_modname = ".".join(processed_components) try: - # Search under the highest package name - # Only relevant if package not already on sys.path - # See https://github.com/python/cpython/issues/89754 for reasoning - # Otherwise can raise bare KeyError: https://github.com/python/cpython/issues/93334 - found_spec = _find_spec_from_path(working_modname, components[0]) + # the path search is not recursive, so that's why we are iterating + # and using the last parent path + found_spec = _find_spec_from_path(working_modname, last_parent) except ValueError: # executed .pth files may not have __spec__ return True + except KeyError: + # Some homonyms might raise KeyErrors + # https://github.com/python/cpython/issues/93334 + # TODO: update if fixed in importlib + # For a tree a > b > a.py + # >>> from importlib.machinery import PathFinder + # >>> PathFinder.find_spec('a.b.a') # same result with path='a' or 'a.b' + # KeyError: 'a.b' + return False + last_parent = working_modname if found_spec is None: return False From b366d8c5a9c3a19971776f34b0cb85dfae5c86ad Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 13:35:16 -0400 Subject: [PATCH 2/7] repair submodule_search_locations --- astroid/interpreter/_import/util.py | 34 ++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py index e14bd4ea6a..c2aeac8de7 100644 --- a/astroid/interpreter/_import/util.py +++ b/astroid/interpreter/_import/util.py @@ -2,8 +2,12 @@ # For details: https://github.com/PyCQA/astroid/blob/main/LICENSE # Copyright (c) https://github.com/PyCQA/astroid/blob/main/CONTRIBUTORS.txt +from __future__ import annotations + +import pathlib import sys from functools import lru_cache +from importlib._bootstrap_external import _NamespacePath from importlib.util import _find_spec_from_path @@ -19,27 +23,41 @@ def is_namespace(modname: str) -> bool: # not, but requires instead that each single parent ('astroid', 'nodes', etc.) # be specced from left to right. processed_components = [] - last_parent = None + last_submodule_search_locations: _NamespacePath | None = None for component in modname.split("."): processed_components.append(component) working_modname = ".".join(processed_components) try: # the path search is not recursive, so that's why we are iterating # and using the last parent path - found_spec = _find_spec_from_path(working_modname, last_parent) + found_spec = _find_spec_from_path( + working_modname, last_submodule_search_locations + ) except ValueError: # executed .pth files may not have __spec__ return True except KeyError: - # Some homonyms might raise KeyErrors + # Intermediate steps might raise KeyErrors # https://github.com/python/cpython/issues/93334 # TODO: update if fixed in importlib - # For a tree a > b > a.py + # For tree a > b > c.py # >>> from importlib.machinery import PathFinder - # >>> PathFinder.find_spec('a.b.a') # same result with path='a' or 'a.b' - # KeyError: 'a.b' - return False - last_parent = working_modname + # >>> PathFinder.find_spec('a.b', ['a']) + # KeyError: 'a' + + # Repair last_submodule_search_locations + if last_submodule_search_locations: + assumed_location = ( + # pylint: disable=unsubscriptable-object + pathlib.Path(last_submodule_search_locations[-1]) + / component + ) + last_submodule_search_locations.append(str(assumed_location)) + continue + + # Update last_submodule_search_locations + if found_spec and found_spec.submodule_search_locations: + last_submodule_search_locations = found_spec.submodule_search_locations if found_spec is None: return False From 9ac2e954acbfc6408327c2fdc7f6999739540ff4 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 13:43:33 -0400 Subject: [PATCH 3/7] py37 --- astroid/interpreter/_import/util.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py index c2aeac8de7..69e9e85131 100644 --- a/astroid/interpreter/_import/util.py +++ b/astroid/interpreter/_import/util.py @@ -49,7 +49,8 @@ def is_namespace(modname: str) -> bool: if last_submodule_search_locations: assumed_location = ( # pylint: disable=unsubscriptable-object - pathlib.Path(last_submodule_search_locations[-1]) + # TODO: py38: directly access last_submodule_search_locations + pathlib.Path(last_submodule_search_locations._path[-1]) / component ) last_submodule_search_locations.append(str(assumed_location)) From 5baddd42b8b0ffa72ef864911ded9c4db595ab1b Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 13:59:12 -0400 Subject: [PATCH 4/7] fixup --- astroid/interpreter/_import/util.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py index 69e9e85131..e228c5aeaa 100644 --- a/astroid/interpreter/_import/util.py +++ b/astroid/interpreter/_import/util.py @@ -47,12 +47,13 @@ def is_namespace(modname: str) -> bool: # Repair last_submodule_search_locations if last_submodule_search_locations: - assumed_location = ( + # TODO: py38: remove except + try: # pylint: disable=unsubscriptable-object - # TODO: py38: directly access last_submodule_search_locations - pathlib.Path(last_submodule_search_locations._path[-1]) - / component - ) + last_item = last_submodule_search_locations[-1] + except TypeError: + last_item = last_submodule_search_locations._recalculate()[-1] + assumed_location = pathlib.Path(last_item) / component last_submodule_search_locations.append(str(assumed_location)) continue From 83b6331d5f87df66414f455e3424d855eacabbf0 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 14:05:04 -0400 Subject: [PATCH 5/7] add test --- tests/unittest_manager.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unittest_manager.py b/tests/unittest_manager.py index d098ca1343..922b78a9bf 100644 --- a/tests/unittest_manager.py +++ b/tests/unittest_manager.py @@ -125,6 +125,9 @@ def test_submodule_homonym_with_non_module(self) -> None: util.is_namespace("tests.testdata.python3.data.parent_of_homonym.doc") ) + def test_module_is_not_namespace(self) -> None: + self.assertFalse(util.is_namespace("tests.testdata.python3.data.all")) + 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") From 97fd7dcf5e51caa7e438f6deba54696f5a42c241 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 15:45:51 -0400 Subject: [PATCH 6/7] add comments --- astroid/interpreter/_import/util.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py index e228c5aeaa..7ef5608f80 100644 --- a/astroid/interpreter/_import/util.py +++ b/astroid/interpreter/_import/util.py @@ -29,9 +29,9 @@ def is_namespace(modname: str) -> bool: working_modname = ".".join(processed_components) try: # the path search is not recursive, so that's why we are iterating - # and using the last parent path + # to incorporate each submodule search path, e.g. a, a/b, a/b/c found_spec = _find_spec_from_path( - working_modname, last_submodule_search_locations + working_modname, path=last_submodule_search_locations ) except ValueError: # executed .pth files may not have __spec__ @@ -53,6 +53,8 @@ def is_namespace(modname: str) -> bool: last_item = last_submodule_search_locations[-1] except TypeError: last_item = last_submodule_search_locations._recalculate()[-1] + # e.g. for failure example above, add 'a/b' and keep going + # so that find_spec('a.b.c', path=['a', 'a/b']) succeeds assumed_location = pathlib.Path(last_item) / component last_submodule_search_locations.append(str(assumed_location)) continue From e6c35dc64eba668f86834d5053cf7dc3befd54a4 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 30 May 2022 16:44:20 -0400 Subject: [PATCH 7/7] better comment --- astroid/interpreter/_import/util.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/astroid/interpreter/_import/util.py b/astroid/interpreter/_import/util.py index 7ef5608f80..3de921b889 100644 --- a/astroid/interpreter/_import/util.py +++ b/astroid/interpreter/_import/util.py @@ -28,8 +28,8 @@ def is_namespace(modname: str) -> bool: processed_components.append(component) working_modname = ".".join(processed_components) try: - # the path search is not recursive, so that's why we are iterating - # to incorporate each submodule search path, e.g. a, a/b, a/b/c + # Both the modname and the path are built iteratively, with the + # path (e.g. ['a', 'a/b', 'a/b/c']) lagging the modname by one found_spec = _find_spec_from_path( working_modname, path=last_submodule_search_locations )