classification
Title: importlib's spec module create algorithm is not exposed
Type: behavior Stage:
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution:
Dependencies: 20383 21581 Superseder:
Assigned To: Nosy List: Arfrever, aronacher, brett.cannon, eric.araujo, eric.snow, flox, ncoghlan, nikicat, tshepang, yselivanov
Priority: normal Keywords: 3.4regression

Created on 2014-04-15 14:24 by aronacher, last changed 2016-01-15 21:35 by brett.cannon. This issue is now closed.

Messages (15)
msg216295 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2014-04-15 14:24
3.4 deprecates load_module on the loaders and now proposes to use create_module (optionally) and exec_module.  Unfortunately for external callers these interfaces are not useful because you need to reimplement _SpecMethods.create and a whole bunch of other stuff for it.

Right now a caller can get hold of _SpecMethods by importing from _frozen_importlib but that seems very unsupported and is obviously entirely not exposed.

Is there a reason those methods are not on the ModuleSpec directly but on a private wrapper?

Example usage:

from types import ModuleType
from importlib.machinery import ModuleSpec, SourceFileLoader
from _frozen_importlib import _SpecMethods

loader = SourceFileLoader('plugin_base', 'my_file.py')
spec = ModuleSpec(name=loader.name, loader=loader)
mod = _SpecMethods(spec).create()

In the absence of _SpecMethods a caller needs to do this:

from importlib.machinery import ModuleSpec, SourceFileLoader

loader = SourceFileLoader('plugin_base', 'my_file.py')
spec = ModuleSpec(name=loader.name, loader=loader)
if not hasattr(loader, 'create_module'):
    module = types.ModuleType(loader.name)
else:
    module = loader.create_module(spec)
module.__loader__ = loader
module.__spec__ = spec
try:
    module.__package__ = spec.parent
except AttributeError:
    pass
loader.exec_module(module)

(And that is not the whole logic yet)
msg216296 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2014-04-15 14:26
On further investigation that is not even enough yet due to the new locking mechanism.  I'm not even sure if exposing _SpecMethods would be enough.
msg216303 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-04-15 15:01
I agree that this is something we need to address in 3.5.  Adding this to 3.4 won't be an option since it would require a new feature.  However, Loader.load_module() is only deprecated (and won't be removed in 3.X), so the current approach will still work until we provide a new approach in 3.5.  The only gap is that now it is possible (even if very unlikely) that in 3.4 you would run into a loader that does not implement load_module(), which would obviously break direct calls to the method. :(

For 3.4 such direct calls in the stdlib to loader.load_module() were replaced with this (mostly):

  spec = importlib.util.spec_from_loader(name, loader)
  # or importlib.util.spec_from_file_location(...)
  methods = _SpecMethods(spec)
  mod = methods.load()

As you already noted, this relies on a current implementation detail of importlib._bootstrap.  We'd rather not encourage use of such private API so providing a simple replacement makes sense.

Futhermore, for 3.5 (in the "soon" timeframe) I'm planning on working on improving the import system abstractions*.  My expectation is that part of that will be exposing most or all of the functionality of _SpecMethods directly.  At the same time I don't want that API to be a distraction.  I think accomplishing both is doable.

So you shouldn't need to worry about create() or anything else.

* Suggestions welcome. :)  You can email me directly (ericsnowcurrently@gmail.com).
msg216307 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-04-15 15:26
Are you after just the module creation/initialization code so you can call exec_module() yourself, Armin, or do you want even more of the algorithm exposed (e.g. the lock stuff)? There are not tons of importlib users to the level of wanting to re-implement import to the level of wanting to control module creation, setting sys.modules manually, etc., so we are constantly trying to figure out how far to go in exposing things without going so far as to make it impossible to make changes.
msg216313 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2014-04-15 15:41
I'm not sure myself what I need right now.  I personally have avoided importlib/imp entirely for my code and I roll with manual module creation because it is most stable between 2.6 - 3.4 but it's getting more complicated to work because of all the new attributes (__package__, __spec__ etc.).

This particular case came from SQLAlchemy I believe (I tried to help Mike Bayer transition his code).

It's true that create() is the wrong function, load() is actually what should have been used there.
msg216314 - (view) Author: Armin Ronacher (aronacher) * (Python committer) Date: 2014-04-15 15:43
Also mostly unrelated importlib now does something I have never seen an ABC do: the ABC has create_module but concrete implementations mostly have that function entirely absent.  That should probably be reconsidered as it's super confusing.
msg216324 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-04-15 15:58
I've opened up #21240 to address the the docs concern.  Thanks for bringing it up. :)
msg219044 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-05-24 14:07
Issue #20383 is tracking adding an API to simply getting the proper module and having it be initialized.
msg219136 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-05-26 06:14
How about this replacement for direct use of Loader.load_module():

# in importlib.util
def load(spec_or_name, /, **kwargs):  # or "load_from_spec"
    if isinstance(spec_or_name, str):
        name = spec_or_name
        if not kwargs:
            raise TypeError('missing loader')
        spec = spec_from_loader(name, **kwargs)
    else:
        if kwargs:
            raise TypeError('got unexpected keyword arguments')
        spec = spec_or_name
    return _SpecMethods(spec).load()

(See a similar proposal for new_module() in msg219135, issue #20383).
msg219158 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-05-26 14:49
I think that's the wrong abstraction(it would be fine in a third-party library, though, that's trying to smooth over 3.3->3.4 transitions). Since importlib.util.find_spec() always returns a spec, then you want something more like::

  def load(spec):
    loader = spec.loader
    if hasattr(loader, 'exec_module'):
        module = importlib.util.whatever_issue_20383_leads_to()
        loader.exec_module(module)
        return module
    else:
        loader.load_module(spec.name)
        return sys.modules[spec.name]

Since this is Python 3.5 code we are talking about you don't have to worry about the find_loader/find_module case as find_spec wraps both of those and normalizes it all to just using specs. You could even tweak it to pass the module in explicitly and in the load_module branch make sure that the module is placed in sys.modules first so it essentially becomes a reload (but as both know that's not exactly the same nor pleasant in certain cases so probably best not exposed that way =).

If this exists in importlib.util then you make import_module() be (essentially)::

  def import_module(name, package):
    fullname = resolve_name(name, package)
    spec = find_spec(fullname)
    return load(spec)
msg219160 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-05-26 14:52
Opened issue #21581 to discuss Armin's point about importlib.abc.Loader.create_module() being there but not being much use since the method is entirely optional.
msg219169 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2014-05-26 18:45
I'm just considering current usage:

  mod = loader.load_module(name)

becomes:

  spec = spec_from_loader(name, loader)
  mod = load(spec)

vs.

  mod = load(name, loader=loader)

I guess I'm torn.  I like how the former forces you to consider specs when dealing with the import system.  On the other hand, I don't want it to be annoying to have to move to this brave new world (that's exactly why we toned down the deprecations).  And on the third hand, perhaps it would be annoying to just a handful of people so we shouldn't sweat it.

So if we end up simplifying, load() could look like this:

def load(spec):  # or "load_from_spec"
    return _SpecMethods(spec).load()

It already takes care of everything we need, including the import lock stuff.
msg219223 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2014-05-27 13:46
I'm not torn so let that settle your torment. =) Considering we are talking about the standard library for a language that has a mantra of "explicit is better than implicit" I think worrying about an added line to very little code since so few people muck with this stuff is enough to not worry about it.
msg255904 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-12-05 00:06
Is there anything left in this issue that hasn't been addressed in Python 3.5?
msg258331 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-01-15 21:35
Due to lack of response, I'm assuming all issues are now addressed.
History
Date User Action Args
2016-01-15 21:35:04brett.cannonsetstatus: pending -> closed

messages: + msg258331
2015-12-05 00:06:06brett.cannonsetstatus: open -> pending

messages: + msg255904
2014-10-15 19:57:47nikicatsetnosy: + nikicat
2014-05-27 13:46:39brett.cannonsetmessages: + msg219223
2014-05-26 18:45:04eric.snowsetmessages: + msg219169
2014-05-26 14:52:25brett.cannonsetmessages: + msg219160
2014-05-26 14:51:45brett.cannonsetdependencies: + Consider dropping importlib.abc.Loader.create_module()
2014-05-26 14:49:25brett.cannonsetdependencies: + Add a keyword-only spec argument to types.ModuleType
messages: + msg219158
2014-05-26 06:14:40eric.snowsetmessages: + msg219136
2014-05-24 14:07:35brett.cannonsetmessages: + msg219044
2014-05-23 20:35:06Arfreversetnosy: + Arfrever
2014-04-16 23:04:28eric.araujosetnosy: + eric.araujo
2014-04-16 11:43:21tshepangsetnosy: + tshepang
2014-04-15 16:24:52floxsetnosy: + flox
type: behavior
2014-04-15 15:58:46eric.snowsetmessages: + msg216324
2014-04-15 15:53:50yselivanovsetnosy: + yselivanov
2014-04-15 15:43:00aronachersetmessages: + msg216314
2014-04-15 15:41:42aronachersetmessages: + msg216313
2014-04-15 15:26:28brett.cannonsetmessages: + msg216307
2014-04-15 15:01:56eric.snowsetversions: + Python 3.5, - Python 3.4
nosy: + eric.snow, brett.cannon, ncoghlan

messages: + msg216303

components: + Library (Lib)
2014-04-15 14:26:36aronachersetmessages: + msg216296
2014-04-15 14:24:41aronachercreate