classification
Title: importlib find_loader returns a loader for a non existent module
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, eric.smith, python-dev, r.david.murray, xdegaye
Priority: normal Keywords:

Created on 2012-11-16 17:32 by xdegaye, last changed 2012-11-17 17:23 by xdegaye. This issue is now closed.

Messages (14)
msg175697 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-16 17:32
Create the following tree:

    foo.py
    mypackage
        __init__.py

and get a loader for the non existent module 'mypackage.foo'.

$ mkdir tmp
$ cd tmp
$ >foo.py
$ mkdir mypackage
$ >mypackage/__init__.py
$ ./python
Python 3.4.0a0 (default:53a7c2226a2b, Nov  9 2012, 16:47:41) 
[GCC 4.3.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import importlib
>>> importlib.find_loader('mypackage').get_filename()
'./mypackage/__init__.py'
>>> importlib.find_loader('mypackage.foo').get_filename()
'./foo.py'
>>>


find_loader should return None in this case.
msg175698 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-16 17:42
And yes, find_loader returns None, when correctly invoked with the
path argument:

>>> importlib.find_loader('mypackage.foo', ['./mypackage/'])
>>>
msg175700 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2012-11-16 18:27
Not necessarily.  The fact that there is nothing to load doesn't mean it isn't the right loader if there *was* something to load.  But I'll leave it to the import experts to say what the expected behavior is.  I'll admit that I can't figure it out from a quick perusal of the docs and PEP, so I'd say that regardless of what the correct behavior is there is at least a doc bug here.
msg175704 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-11-16 18:56
Everything is working as expected; you left out the path argument::

  importlib.find_loader('package.foo', ['package'])

This works the way it does because otherwise we would have to stat every single time from the top level on down and that is extremely costly.
msg175707 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-16 19:30
Maybe find_loader could check its parameters, notice that the name is a
dotted name, that path is None and in this case, not return a loader ?
msg175708 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-11-16 19:34
That won't work as frozen and builtin modules don't care about the path.
msg175710 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-16 19:47
> Not necessarily.  The fact that there is nothing to load doesn't
> mean it isn't the right loader if there *was* something to load.

But it is not even the right loader if there *was* something to load,
as get_filename() returns './foo.py' which is wrong even if
mypackage/foo.py had existed.  It should have returned
'./mypackage/foo.py' to be an acceptable loader for a would-be
mypackage.foo module.
msg175716 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-16 21:41
If one would want to fix this, one way to do it could be to change the
following two methods of the PathFinder class such that:

    find_module() does not set path to sys.path when its path argument
    is None, so as to keep this information for _get_loader() to
    process it later

    _get_loader() sets path to sys.path when its path argument is None
    and returns a None loader when a loader has been found by a finder
    and:
        - this finder is a FileFinder instance
        - the _get_loader() path argument is None
        - the module name is a dotted name
msg175718 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-11-17 01:25
Feel free to submit a patch, but I'm personally not motivated enough to try to change this on my own.
msg175719 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2012-11-17 01:28
It might be more motivating if you (Xavier de Gaye) could let us know of a real-world problem caused by this behavior.
msg175734 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-17 12:15
I was bitten by this behavior while, new to the importlib library, I
was trying to understand if one has to call recursively find_loader
for a dotted module name (in the way it must be done when using
imp.find_module), since the documentation on find_loader is not
clear. My test environment happened to be at that time:

    foo.py
    mypackage
        __init__.py
        foo.py

and at first I could not understand why find_loader('mypackage.foo')
was returning a loader, while find_loader('logging.handlers') was
returning None.

It is fine with me to consider that this behavior does not have
to be changed and to close this discussion.
msg175740 - (view) Author: Roundup Robot (python-dev) Date: 2012-11-17 14:33
New changeset 358be7742377 by Brett Cannon in branch '3.3':
Issue #16489: Make it clearer that importlib.find_loader() requires
http://hg.python.org/cpython/rev/358be7742377

New changeset ba1d7447bd1b by Brett Cannon in branch 'default':
Merge fix for #16489 from 3.3
http://hg.python.org/cpython/rev/ba1d7447bd1b
msg175743 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-11-17 14:39
I clarified the wording in 3.3 and default. I also opened issue #16492 to add a keyword-only argument to find_loader() to have it import parent packages for you if you so desire.
msg175783 - (view) Author: Xavier de Gaye (xdegaye) * (Python committer) Date: 2012-11-17 17:23
Thanks, this is great!
History
Date User Action Args
2012-11-17 17:23:15xdegayesetmessages: + msg175783
2012-11-17 14:39:43brett.cannonsetresolution: not a bug -> fixed
messages: + msg175743
stage: resolved
2012-11-17 14:33:47python-devsetnosy: + python-dev
messages: + msg175740
2012-11-17 12:15:21xdegayesetmessages: + msg175734
2012-11-17 01:28:57eric.smithsetmessages: + msg175719
2012-11-17 01:25:06brett.cannonsetmessages: + msg175718
2012-11-16 21:41:26xdegayesetmessages: + msg175716
2012-11-16 19:47:09xdegayesetmessages: + msg175710
2012-11-16 19:34:56brett.cannonsetmessages: + msg175708
2012-11-16 19:30:24xdegayesetmessages: + msg175707
2012-11-16 18:56:10brett.cannonsetstatus: open -> closed
resolution: not a bug
messages: + msg175704
2012-11-16 18:27:03r.david.murraysetnosy: + r.david.murray, eric.smith, brett.cannon
messages: + msg175700
2012-11-16 17:42:55xdegayesetmessages: + msg175698
2012-11-16 17:32:11xdegayecreate