GH-119668: expose importlib.machinery.NamespacePath #119669
Conversation
Signed-off-by: Filipe Laíns <[email protected]>
FFY00
requested review from
brettcannon,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Misc/NEWS.d/next/Library/2024-05-28-17-14-30.gh-issue-119668.RrIGpn.rst
Outdated
Show resolved
Hide resolved
|
When you're done making the requested changes, leave the comment: |
Co-authored-by: Bénédikt Tran <[email protected]>
|
The docs failure is related to the fact that you did not yet merge main into your branch. Since I don't like updating branches of people, I'll let you do it yourself. |
| @@ -14,6 +14,7 @@ | |||
| from ._bootstrap_external import ExtensionFileLoader | |||
| from ._bootstrap_external import AppleFrameworkLoader | |||
| from ._bootstrap_external import NamespaceLoader | |||
| from ._bootstrap_external import NamespacePath | |||
There was a problem hiding this comment.
Contrary to the title and what's new entry, this doesn't expose importlib.NamespacePath, but importlib.machinery.NamespacePath:
>>> from importlib import NamespacePath
Traceback (most recent call last):
File "<python-input-0>", line 1, in <module>
from importlib import NamespacePath
ImportError: cannot import name 'NamespacePath' from 'importlib' (...)
>>> from importlib.machinery import NamespacePath
>>> Aside from what you intended, I'm not sure that this isn't actually better. importlib.NamespacePath seems a little odd to me, and while something like importlib.abc might be better, but this isn't really an ABC. So importlib.machinery.NamespacePath seems fine. Care to chime in, @brettcannon ?
There was a problem hiding this comment.
Yes, sorry! I am okay with either namespace. From the lack of feedback, I think it should be okay to proceed with importlib.machinery.NamespacePath.
FFY00
changed the title
Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
|
|
||
| For top-level modules, the parent module's path is :data:`sys.path`. | ||
|
|
||
|
|
There was a problem hiding this comment.
Can we use .. versionadded:: next + what's new entry? While this only exposes a private class, I think users should be notified more than just with a blurb.
| @@ -1147,12 +1150,15 @@ def append(self, item): | |||
| self._path.append(item) | |||
|
|
|||
|
|
|||
| _NamespacePath = NamespacePath # for backwards compatibility | |||
There was a problem hiding this comment.
Should we make it a deprecated alias? (and emit a warning if we were to access the private name? that could help transition from the private to the public name).
|
|
||
| It uses the module *name* to find its parent module, and from there it looks | ||
| up the parent's :attr:`module.__path__`. When this changes, the module's own | ||
| path is recomputed, using *path_finder*. The initial value is set to *path*. |
There was a problem hiding this comment.
Can we perhaps mention the type of the path finder? (namely using some class Sphinx directive).
Btw, when you say that the initial value is set to path it seems that you are talking about the initial value of the path finder.
In addition, maybe mention that invalidating the caches forces the path to be recomputed (it might not necessarily be the case that the module parent's path has changed).
Finally, could you document the public methods of this new class? For instance there is no insert() method nor remove() as for lists.
I think an example of how to use it could be useful though I don't know whether we want to burden the docs even more.
_NamespacePath#119668