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

LazyLoader rejecting use of SourceFileLoader #70374

Closed
brettcannon opened this issue Jan 23, 2016 · 13 comments
Closed

LazyLoader rejecting use of SourceFileLoader #70374

brettcannon opened this issue Jan 23, 2016 · 13 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@brettcannon
Copy link
Member

BPO 26186
Nosy @brettcannon, @ncoghlan, @pitrou, @ethanfurman, @ericsnowcurrently

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/brettcannon'
closed_at = <Date 2016-06-25.17:59:11.028>
created_at = <Date 2016-01-23.19:28:00.225>
labels = ['type-bug', 'library']
title = 'LazyLoader rejecting use of SourceFileLoader'
updated_at = <Date 2016-06-25.17:59:11.026>
user = 'https://github.com/brettcannon'

bugs.python.org fields:

activity = <Date 2016-06-25.17:59:11.026>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2016-06-25.17:59:11.028>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2016-01-23.19:28:00.225>
creator = 'brett.cannon'
dependencies = []
files = []
hgrepos = []
issue_num = 26186
keywords = []
message_count = 13.0
messages = ['258873', '258875', '259157', '259584', '259585', '259586', '259587', '259588', '260592', '260593', '263524', '269244', '269245']
nosy_count = 7.0
nosy_names = ['brett.cannon', 'ncoghlan', 'kdart', 'pitrou', 'ethan.furman', 'python-dev', 'eric.snow']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26186'
versions = ['Python 3.5', 'Python 3.6']

@brettcannon
Copy link
Member Author

It was privately reported to me that importlib.util.LazyLoader rejects using importlib.machinery.SourceFileLoader (or at least _frozen_importlib.SourceFileLoader). At least a test should be added for LazyLoader to make sure it will happily accept importlib.machinery.SourceFileLoader, and if it rejects the one from _frozen_importlib, tweak the test to accept loaders from there as well.

@brettcannon brettcannon self-assigned this Jan 23, 2016
@brettcannon brettcannon added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jan 23, 2016
@brettcannon
Copy link
Member Author

One way to possibly improve this is to remove the create_module() check and replace it with a warning if create_module() doesn't return None. Another option is to use an assert statement. The final option is to just drop the check entirely, although that can make debugging difficult.

@brettcannon
Copy link
Member Author

I think I'm liking the following approach:

  if __debug__:
    hopefully_None = loader.create_module(spec)
    if hopefully_None is not None:
        warnings.warn("watch out!", ImportWarning)

@pitrou
Copy link
Member

pitrou commented Feb 4, 2016

Just stumbled on this very issue while trying to use LazyLoader.

@brettcannon
Copy link
Member Author

My current plan is to simply remove the check in 3.5 -- the docs say it's ignored so I don't think it will hurt anything -- and add the warning I proposed in 3.6. What do you think, Antoine?

@pitrou
Copy link
Member

pitrou commented Feb 4, 2016

By the way does this mean the LazyLoader won't work with ExtensionFileLoader? That would reduce its usefulness quite a bit.

@pitrou
Copy link
Member

pitrou commented Feb 4, 2016

I don't know what the impact of the error is, but it seems like at least the default loader classes should be able to work with LazyLoader...

@brettcannon
Copy link
Member Author

You're right, it won't work with extension modules based on how ExtensionFileLoader is structured.

@python-dev
Copy link
Mannequin

python-dev mannequin commented Feb 21, 2016

New changeset 9f1e680896ef by Brett Cannon in branch '3.5':
Issue bpo-26186: Remove an invalid type check in
https://hg.python.org/cpython/rev/9f1e680896ef

New changeset 86fc6cdd65de by Brett Cannon in branch 'default':
Merge for issue bpo-26186
https://hg.python.org/cpython/rev/86fc6cdd65de

@brettcannon
Copy link
Member Author

This has been resolved by removing the check (the docs have always said the method was ignored, so that will just continue). I also did separate commit to list that BuiltinImporter and ExtensionFileLoader won't work (they would need to be updated to return a module subclass to work).

I'm leaving this issue open to decide if I want to add an explicit check in importlib.util.LazyLoader.create_module() to verify that None is returned in Python 3.5 (I probably will, but I want to think about on it for a little bit).

@brettcannon
Copy link
Member Author

I actually think this can be generalized thanks to some tweaks made in Python 3.6 which will lead to it working with extension modules.

I need to double-check, but I think I can:

  • Eliminate the _Module class (and thus have _LazyModule inherit types.ModuleType directly)
  • Call create_module() and see if it returns something
  • Use spec.loader_state to store the original value of module.__class__ (probably be simply storing __class__ in the dict of values)
  • Just blindly try to set module.__class__ to _LazyModule and let the exception propagate if it fails
  • In _LazyModule.__getattribute__, temporarily assign the type to types.ModuleType to shut off __getattribute__() before assigning the proper value

@python-dev
Copy link
Mannequin

python-dev mannequin commented Jun 25, 2016

New changeset 7af4b3ad75b7 by Brett Cannon in branch 'default':
Issue bpo-26186: Remove the restriction that built-in and extension
https://hg.python.org/cpython/rev/7af4b3ad75b7

@brettcannon
Copy link
Member Author

The restriction of lazily loading built-ins and extensions is now lifted!

@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
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants