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

Fix PathEntryFinder / MetaPathFinder in editable_wheel #4278

Merged
merged 4 commits into from
Apr 21, 2024

Conversation

abravalheri
Copy link
Contributor

@abravalheri abravalheri commented Mar 13, 2024

It seems that the import machinery skips PathEntryFinder when trying to locate nested namespaces, if the sys.path_hook item corresponding to the finder cannot be located in the submodule locations of the parent namespace.

Summary of changes

We should (probably) add the PATH_PLACEHOLDER to the namespace spec , so that the PathEntryFinder is triggered.

This PR also add some type hints to the template, because it helped to debug some type errors.

Closes #4248 (likely)

Pull Request Checklist

Sorry, something went wrong.

@abravalheri

This comment was marked as outdated.

@abravalheri abravalheri force-pushed the fix-editable-namespaces branch from e2e2b79 to 8293a81 Compare April 16, 2024 18:32
@abravalheri abravalheri marked this pull request as ready for review April 16, 2024 18:56
@abravalheri abravalheri force-pushed the fix-editable-namespaces branch from 88e40e2 to c540312 Compare April 16, 2024 19:49
It seems that the import machinery skips PathEntryFinder when trying to
locate nested namespaces, if the `sys.path_hook` item corresponding to
the finder cannot be located in the submodule locations of the parent
namespace.

This means that we should probably always add the PATH_PLACEHOLDER to
the namespace spec.

This PR also add some type hints to the template, because it helped to
debug some type errors.
@abravalheri abravalheri force-pushed the fix-editable-namespaces branch from c540312 to 2b7ea60 Compare April 21, 2024 07:51
@abravalheri abravalheri merged commit a3d0be5 into pypa:main Apr 21, 2024
21 checks passed
@abravalheri abravalheri deleted the fix-editable-namespaces branch April 21, 2024 09:11
@@ -530,11 +530,20 @@ def __call__(self, wheel: "WheelFile", files: List[str], mapping: Dict[str, str]

name = f"__editable__.{self.name}.finder"
finder = _normalization.safe_identifier(name)
return finder, name, mapping, namespaces_

def get_implementation(self) -> Iterator[Tuple[str, bytes]]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why hide the true type of Generator[tuple[str, bytes], None, None] here? Is there a reason for only wanting to interact with an Iterator ?

Copy link
Contributor Author

@abravalheri abravalheri May 21, 2024

Choose a reason for hiding this comment

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

My personal opinion is that Generator[tuple[str, bytes], None, None] is a beast/convoluted to understand...
Iterator[Tuple[str, bytes]] is clearer and the reader can get immediately how they are supposed use/call the function.

(also, we don't use the "coroutine" abilities of the "generator"1, i.e. there is no generator.send, so I don't think is relevant to have the explicit generator type there).

Footnotes

  1. Python nomenclature with the generator vs coroutine is weird... Here I mean coroutine as in other languages, without the weird async that ended up messed in the Python concepts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks! Just double checking the intention since I'll need to update the typeshed stubs for version 70

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.

[BUG] package-dir with subpackages (namespaces) does not work with editable install
2 participants