Skip to content

Allow namespace packages in _EditableFinder #4954

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+5 −0
Draft

Allow namespace packages in _EditableFinder #4954

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions setuptools/command/editable_wheel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,6 +836,11 @@ def _find_spec(cls, fullname: str, candidate_path: Path) -> ModuleSpec | None:
for candidate in chain([init], candidates):
if candidate.exists():
return spec_from_file_location(fullname, candidate)
if candidate_path.is_dir():
# treat it like a PEP 420 namespace package
spec = ModuleSpec(fullname, None, is_package=True)
spec.submodule_search_locations = [str(candidate_path)]
return spec
return None
Comment on lines +839 to 844
Copy link
Contributor

Choose a reason for hiding this comment

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

Would something like the following make sense?

Suggested change
if candidate_path.is_dir():
# treat it like a PEP 420 namespace package
spec = ModuleSpec(fullname, None, is_package=True)
spec.submodule_search_locations = [str(candidate_path)]
return spec
return None
return _EditableNamespaceFinder.find_spec(fullname)

(or some kind of refactoring on _EditableNamespaceFinder so that we reuse the same logic?)

Previously when I was working with namespaces, I found that sometimes the import machinery ignores the ModuleSpec object returned by the finders and create a completely new one.
For some cases I found that I had to use a sys.path_hooks instead of sys.meta_path, that is why I implemented the namespace handling in _EditableNamespaceFinder. But it seems that it was not enough 😅. I believe that the relevant portion of the code is
https://github.com/python/cpython/blob/580888927c8eafbf3d4c6be9144684117f0a96ca/Lib/importlib/_bootstrap_external.py#L1259-L1285.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe. I briefly considered using the _EditableNamespaceFinder, but it also felt like that finder had too much behavior that was maybe unneeded for these (ns-package-in-simple-package) packages. Because the parent simple package can't have multiple paths, that also means there's no possibility of this child ns package having multiple paths. In the example, test has one home, so test.b will only ever have one home.

Although, now that I think about it, since Setuptools is doing something special here, and constructing packages virtually from various sources, it's conceivable that test.b could in fact have multiple paths. That is, test.b.foo could come from one path while test.b.bar or test.b.c comes from another. I don't think that's possible given the current package_dir implementation that test.b could ever have modules in multiple paths, but it seems perfectly possible that package_dir = {'test.b.c': 'some path'} could extend the paths and namespace of test.b. And in that case, maybe reusing the functionality of _EditableNamespaceFinder makes sense. I also like the elegance of fewer lines of code. I'll give it a try.



Expand Down
Loading