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

Loader for namespace packages #79854

Closed
ronaldoussoren opened this issue Jan 6, 2019 · 18 comments
Closed

Loader for namespace packages #79854

ronaldoussoren opened this issue Jan 6, 2019 · 18 comments
Assignees
Labels
3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@ronaldoussoren
Copy link
Contributor

BPO 35673
Nosy @warsaw, @brettcannon, @ronaldoussoren, @ericvsmith, @ericsnowcurrently, @FFY00
PRs
  • bpo-35673: Add a public alias for namespace package __loader__ attribute #29049
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/warsaw'
    closed_at = <Date 2021-10-20.21:08:27.267>
    created_at = <Date 2019-01-06.13:23:22.986>
    labels = ['type-bug', 'library', '3.11']
    title = 'Loader for namespace packages'
    updated_at = <Date 2021-10-20.21:09:31.542>
    user = 'https://github.com/ronaldoussoren'

    bugs.python.org fields:

    activity = <Date 2021-10-20.21:09:31.542>
    actor = 'barry'
    assignee = 'barry'
    closed = True
    closed_date = <Date 2021-10-20.21:08:27.267>
    closer = 'barry'
    components = ['Library (Lib)']
    creation = <Date 2019-01-06.13:23:22.986>
    creator = 'ronaldoussoren'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 35673
    keywords = ['needs review']
    message_count = 18.0
    messages = ['333111', '333122', '333141', '333200', '333211', '333213', '333214', '397672', '397674', '404249', '404258', '404259', '404322', '404365', '404509', '404513', '404526', '404527']
    nosy_count = 7.0
    nosy_names = ['barry', 'brett.cannon', 'ronaldoussoren', 'eric.smith', 'eric.snow', 'FFY00', 'fwahhab']
    pr_nums = ['29049']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue35673'
    versions = ['Python 3.11']

    @ronaldoussoren
    Copy link
    Contributor Author

    The documentation for import lib.machinery.ModuleSpec says that the attribute "loader" should be None for namespace packages (see <https://docs.python.org/3/library/importlib.html#importlib.machinery.ModuleSpec\>)

    In reality the loader for namespace packages is an instance of a private class, and that class does not conform to the importlib.abc.Loader ABC.

    To reproduce:

    • Create and empty directory "namespace"
    • (Optionally) create an empty "module.py" in that directory
    • Start a python shell and follow along:
    Python 3.7.2 (v3.7.2:9a3ffc0492, Dec 24 2018, 02:44:43) 
    [Clang 6.0 (clang-600.0.57)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import namespace
    >>> namespace.__loader__
    <_frozen_importlib_external._NamespaceLoader object at 0x104c7bdd8>
    >>> import importlib.abc
    >>> isinstance(namespace.__loader__, importlib.abc.Loader)
    False
    >>> import importlib.util
    >>> importlib.util.find_spec('namespace')
    ModuleSpec(name='namespace', loader=<_frozen_importlib_external._NamespaceLoader object at 0x104c7bdd8>, submodule_search_locations=_NamespacePath(['/Users/ronald/Projects/pyobjc-hg/modulegraph2/namespace']))

    Note how "namespace" has an attribute named "__loader__" that is not None, and the same is true for the ModuleSpec found using importlib.util.find_spec. The loader does not claim to conform to any Loader ABC (but provides all methods required for conformance to the InspectLoader ABC)

    I'm not sure if this should be two issues:

    1. Documentation doesn't match behaviour

    2. The loader for namespace packages isn't registered with the relevant ABCs

    P.S. the latter is also true for zipimport.zipimporter.

    @ronaldoussoren ronaldoussoren added 3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 6, 2019
    @warsaw
    Copy link
    Member

    warsaw commented Jan 6, 2019

    On the first point, I'd categorize this as a documentation bug, and in fact, it's inconsistent with the language reference, which doesn't have the same language:

    https://docs.python.org/3/reference/import.html#__loader__

    On the second point, it probably does make sense to register the ABC.

    @ronaldoussoren
    Copy link
    Contributor Author

    @barry: I agree on both.

    Do you know why the namespace package loader lies about the source and code? Both .get_source() and .get_code() return a value that isn't None.

    And likewise: Why is the namespace package loader a private class, other loaders are exposed in importlib.machinery? This makes it hard to detect PEP-420 style namespace packages without relying on private APIs, esp. combined with the behaviour of .get_source() and .get_code().

    @warsaw
    Copy link
    Member

    warsaw commented Jan 8, 2019

    On Jan 7, 2019, at 03:16, Ronald Oussoren <report@bugs.python.org> wrote:

    Do you know why the namespace package loader lies about the source and code? Both .get_source() and .get_code() return a value that isn't None.

    And likewise: Why is the namespace package loader a private class, other loaders are exposed in importlib.machinery? This makes it hard to detect PEP-420 style namespace packages without relying on private APIs, esp. combined with the behaviour of .get_source() and .get_code().

    I don’t remember the history of this. I wonder if Brett or Eric have any additional insights.

    @ericvsmith
    Copy link
    Member

    Namespace packages (PEP-420) predate ModuleSpec (PEP-451). So, I think this probably happened when 451 was implemented. Maybe Eric Snow recalls?

    I say this without having looked at it very deeply.

    As to why the namespace package loader is a private class: it never occurred to me someone would care about inspecting it.

    @ronaldoussoren
    Copy link
    Contributor Author

    The background for all of this: I'm currently rewriting modulegraph (https://pypi.org/project/modulegraph/) to use importlib instead of its own implementation of the import mechanism.

    I currently detect PEP-420 style namespace packages, but I'm not sure if I really need that information. With the current behaviour of the namespace loader I probably do, because I'd otherwise end up with creating an __init__.py for these packages when I use the generated module graph in py2app to copy modules into an application bundle.

    @ericvsmith
    Copy link
    Member

    I think exposing _NamespaceLoader as NamespaceLoader and registering the ABC make sense. That would make this in to a feature request for 3.8.

    @fwahhab
    Copy link
    Mannequin

    fwahhab mannequin commented Jul 16, 2021

    Not sure if it's proper etiquette to bump issues on the tracker, but is there any interest in this issue for 3.11?

    @ericvsmith
    Copy link
    Member

    I think a PR with tests would be a good first step.

    @warsaw
    Copy link
    Member

    warsaw commented Oct 18, 2021

    I'm going to take a look at this during the Python core sprint.

    @warsaw warsaw self-assigned this Oct 18, 2021
    @warsaw warsaw added 3.9 only security fixes 3.10 only security fixes 3.11 bug and security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Oct 18, 2021
    @warsaw
    Copy link
    Member

    warsaw commented Oct 19, 2021

    First crack at a PR for this issue.

    @warsaw
    Copy link
    Member

    warsaw commented Oct 19, 2021

    Since the documentation problem reported here has since been fixed, and really all that's left is to expose NamespaceLoader publicly and register it with the abc, this is technically a new feature so it can't be backported. Thus, targeting only 3.11.

    @warsaw warsaw removed 3.9 only security fixes 3.10 only security fixes labels Oct 19, 2021
    @brettcannon
    Copy link
    Member

    Should we register with the ABC or is it time to do proper typing.Protocol classes and have the ABCs inherit from those?

    @warsaw
    Copy link
    Member

    warsaw commented Oct 19, 2021

    I don't know. What benefit would be gained? That should probably be a separate issue/PR in either case.

    @brettcannon
    Copy link
    Member

    What benefit would be gained?

    The ABCs are broader than what the import system actually requires due to their helper methods. So for typing purposes they are actually not a perfect fit.

    That should probably be a separate issue/PR in either case.

    https://bugs.python.org/issue38782 and I was trying to rope you into doing the work. 😁

    @ericsnowcurrently
    Copy link
    Member

    On Mon, Jan 7, 2019 at 11:41 PM Eric V. Smith <report@bugs.python.org> wrote:

    Namespace packages (PEP-420) predate ModuleSpec (PEP-451). So, I think this probably happened when 451 was implemented. Maybe Eric Snow recalls?

    PEP-451 talks about this a little
    (https://www.python.org/dev/peps/pep-0451/#namespace-packages):

    """
    Currently a path entry finder may return (None, portions) from
    find_loader() to indicate it found part
    of a possible namespace package. To achieve the same effect,
    find_spec() must return a spec with
    "loader" set to None (a.k.a. not set) and with
    submodule_search_locations set to the same portions
    as would have been provided by find_loader(). It's up to PathFinder
    how to handle such specs.
    """

    @warsaw
    Copy link
    Member

    warsaw commented Oct 20, 2021

    New changeset 876fc7f by Barry Warsaw in branch 'main':
    bpo-35673: Add a public alias for namespace package __loader__ attribute (bpo-29049)
    876fc7f

    @warsaw warsaw closed this as completed Oct 20, 2021
    @warsaw
    Copy link
    Member

    warsaw commented Oct 20, 2021

    On Oct 20, 2021, at 11:27, Brett Cannon <report@bugs.python.org> wrote:

    > That should probably be a separate issue/PR in either case.

    https://bugs.python.org/issue38782 and I was trying to rope you into doing the work. 😁

    Ha! You should have nosied me then :D - but anyway I’m following that ticket now so we’ll see.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 bug and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants