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.

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

improvement: submodule_search_locations is not always a list #12145

Closed
wants to merge 1 commit into from

Conversation

bzoracler
Copy link
Contributor

@bzoracler bzoracler commented Jun 16, 2024

Despite the CPython documentation for importlib.machinery.ModuleSpec.submodule_search_locations, this attribute may not be a list - and for CPython at least it is a importlib._bootstrap_external._NamespacePath object for namespace packages. MRE:

>>> from importlib.util import find_spec
>>> find_spec("existing_namespace_package_name").spec.submodule_search_locations
_NamespacePath(['/path/to/existing_namespace_package_name'])

The type of .submodule_search_locations mirrors that of types.ModuleType.__path__.

__path__: MutableSequence[str]

(See also #6200)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pytest (https://github.com/pytest-dev/pytest)
+ src/_pytest/assertion/rewrite.py:130: error: Argument "submodule_search_locations" to "spec_from_file_location" has incompatible type "MutableSequence[str] | None"; expected "list[str] | None"  [arg-type]

@bzoracler
Copy link
Contributor Author

bzoracler commented Jun 16, 2024

This change causes an issue with importlib._bootstrap_external.spec_from_file_location - specifically on this line:

https://github.com/python/cpython/blob/main/Lib/importlib/_bootstrap_external.py#L892

        if spec.submodule_search_locations == []:

Empty _NamespacePath containers don't override __eq__ (so they fall back to object.__eq__ -> is), so this line will always compare False when passed a namespace package's submodule_search_locations.


_NamespacePath also doesn't seem to fulfil the MutableSequence interface - it doesn't have e.g. an insert method. This PR isn't the way to go - I think we need a proper _NamespacePath type.

@bzoracler bzoracler marked this pull request as draft June 16, 2024 22:24
@bzoracler
Copy link
Contributor Author

python/cpython#119668 looks to expose _NamespacePath as public API. Since importlib._bootstrap_external isn't a public module, we can wait to see where _NamespacePath ends up being exposed (importlib or importlib.machinery) and reflect that in the stubs.

@bzoracler bzoracler closed this Jun 19, 2024
@bzoracler bzoracler deleted the patch-1 branch June 19, 2024 06:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant