This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Title: 'import my_pkg.__init__' creates duplicate modules
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.3
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: Aaron.Meurer, Arfrever, Ronan.Lamy, brett.cannon, python-dev
Priority: normal Keywords: patch

Created on 2012-05-28 18:52 by Ronan.Lamy, last changed 2022-04-11 14:57 by admin. This issue is now closed.

File name Uploaded Description Edit
my_pkg.tar.bz2 Ronan.Lamy, 2012-05-28 18:52
1.patch Ronan.Lamy, 2012-06-01 11:39 review
2.patch Ronan.Lamy, 2012-06-01 20:33 review
Messages (18)
msg161799 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-05-28 18:52
If an file contains relative imports, doing 'import my_pkg.__init__' or calling __import__('my_pkg.__init__') creates duplicate versions of the relatively imported modules, which (I believe) causes cryptic errors in some cases (cf. the metaclass issue in ).

More precisely, with my_pkg/ containing (see attachment for the full setup):

from .module1 import a
from my_pkg.module2 import b

I get:

Python 3.3.0a3+ (default:0685f51e9891, May 27 2012, 02:22:12) 
[GCC 4.6.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import my_pkg.__init__
>>> import sys
>>> sorted(name for name in sys.modules if name.startswith('my_'))
['my_pkg', 'my_pkg.__init__', 'my_pkg.__init__.module1', 'my_pkg.module1', 'my_pkg.module2']

Note the bogus 'my_pkg.__init__.module1' entry. For reference, in Python 3.2, the last line is:

['my_pkg', 'my_pkg.__init__', 'my_pkg.module1', 'my_pkg.module2']

NB: calling __import__('my_pkg.__init__') might seem odd (I didn't actually expect it to work on any Python version), but doctest apparently relies on it to test files.
msg161809 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-05-28 21:28
You can't have it both ways. If you explicitly import __init__ then it becomes just another module to Python, but you still need the implicit package module (i.e. without the __init__ name) for everything else to work since so much of the import system relies on the name of the module itself. If one chooses to work around the import system by doing stuff that is non-standard you will get quirky results like this.

Closing as "won't fix" as it isn't worth the code complication to support such an edge case.
msg161825 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-05-29 01:23
my_pkg.__init__ isn't treated as just another module but as a package. That is the reason why the bogus my_pkg.__init__.module1 is created, I guess:

>>> import sys
>>> sorted(name for name in sys.modules if name.startswith('my_'))
>>> import my_pkg.__init__
>>> sorted(name for name in sys.modules if name.startswith('my_'))
['my_pkg', 'my_pkg.__init__', 'my_pkg.__init__.module1', 'my_pkg.module1', 'my_pkg.module2']
>>> sys.modules['my_pkg.module1'].__package__
>>> sys.modules['my_pkg.__init__'].__package__

I agree that importing __init__ is a hack, but the way 3.3 reacts to it is nasty, because it can cause a whole application to be executed multiple times.
msg161826 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-05-29 01:25
Grmf. I didn't mean to change the status.
msg161889 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-05-29 14:38
If you can come up with a patch to fix this in a way that is not messy then I will be happy to look at it.
msg161914 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-05-29 20:37
Well, I can try. I have trouble wrapping my head around all the finders and loaders, but I'm slowly understanding what does what.

What I don't know is what the expected behaviour is. Should my_pkg.__init__ be allowed to exist as a separate module (that's backwards-compatible but seems wrong)? Can it exist as a synonym of my_pkg? Should the import attempt raise an ImportError?
msg161958 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-05-30 14:31
If you directly import __init__ then it would just be a module within the package (the "magic" of packages should stay with the implicit interpretation of __init__).
msg161964 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-05-30 18:13
Reverting to the previous behaviour, then? OK.

As I understand it, the issue comes from a DRY violation: both FileFinder.find_loader() and _LoaderBasics.is_package() have their own notion of what is a package and they disagree. Since the finder needs to know what is a package, I think that the loader should be told whether it's a package or not instead of trying to guess.
msg162007 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-05-31 15:30
I see what you mean about the discrepancy, but you don't need to complicate the constructor to get the desired result. If you have is_package() check if the module name ends in __init__ to skip the package check and just say False (e.g. only if the path ends in __init__ but the module name does not) then you will get your desired semantics out of is_package (since this is what find_loader() is doing).
msg162016 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-05-31 19:46
That would force the Loaders to know how to convert a module name into a file path, which isn't the case now since FileLoader.get_filename() is just a shim that returns self.path. So I'd rather add an optional argument to FileLoader. Actually, I feel that the clean solution would be to have packages be a separate Loader class, but that would be a significant change...
msg162068 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-06-01 11:39
Here's a preliminary patch, without tests or documentation, implementing my approach (adding an optional is_package=False to FileLoader.__init__()).

Next question (quite independent of the chosen implementation): how should this be tested?
msg162075 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-06-01 14:38
I think you misunderstood what I was suggesting as a solution: there is no conversion of module name to file name. It would simply be::

fullname.rpartition('.')[2] == os.path.splitext(os.path.split(self.path)[1])[0]

for path comparison. But honestly since __init__ is the only special case here, is_package() could just do its current check and ``fullname.rpartition('.')[2] != '__init__'``.

And a problem with your patch is that it is only for FileLoader, which precludes SourceLoader from getting the beneficial fix for the issue and thus leaves out all other loaders which might use an alternative storage mechanism than files (e.g. zip files).
msg162083 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-06-01 17:34
Your second solution is simple and solves the problem, so I guess that's it, then. 

Still, I don't really understand the purpose of SourceLoader, which makes me uneasy about the implementation of _LoaderBasics.is_package(). It seems to be meant to load an arbitrary resource yet it assumes that get_filename returns an actual file path, so I don't see how it could be used to import from a URL, for instance.
msg162103 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-06-01 20:06
I have yet to see anyone use a URL loader in serious code beyond people just using it as an example. What I do see is lots of custom zipfile importers and thus for the majority if people this will continue to make sense. And even with a URL loader, get_filename would return a URL who probably has something like which would still work as expected. If someone chooses to break from expectations they can just overload is_package().
msg162106 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-06-01 20:33
OK, that makes sense pragmatically.

Here's the patch then. I wasn't sure where to put the test, it doesn't actually have much in common with the rest of Lib/importlib/test/import_/ but the name fits, so...
msg162152 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-06-02 17:29
Thanks for the patch, Ronan! The fix seems fine and I will have a more thorough look at the test later and figure out where it should go (probably only going to worry about testing is_package() directly since that was the semantic disconnect). I will also update the docs when I commit the patch.

And as a matter of procedure, Ronan, can you submit a contributor agreement?
msg162240 - (view) Author: Ronan Lamy (Ronan.Lamy) * Date: 2012-06-03 23:31
I'm not sure that it's enough to test is_package() because that only involves the loader and not the interaction between it and FileFinder. That's the reason why my test works at a higher level.

BTW, I sent the contributor agreement.
msg162942 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-06-16 00:01
New changeset 240b7467e65c by Brett Cannon in branch 'default':
Issue #14938: now takes the
Date User Action Args
2022-04-11 14:57:30adminsetgithub: 59143
2012-06-16 00:01:40brett.cannonsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-06-16 00:01:22python-devsetnosy: + python-dev
messages: + msg162942
2012-06-03 23:31:21Ronan.Lamysetmessages: + msg162240
2012-06-02 17:29:00brett.cannonsetmessages: + msg162152
stage: test needed -> patch review
2012-06-01 20:33:00Ronan.Lamysetfiles: + 2.patch

messages: + msg162106
2012-06-01 20:06:53brett.cannonsetstatus: closed -> open
messages: + msg162103

assignee: brett.cannon
resolution: wont fix -> (no value)
stage: test needed
2012-06-01 17:34:03Ronan.Lamysetmessages: + msg162083
2012-06-01 14:38:28brett.cannonsetmessages: + msg162075
2012-06-01 11:39:53Ronan.Lamysetfiles: + 1.patch
keywords: + patch
messages: + msg162068
2012-05-31 19:46:20Ronan.Lamysetmessages: + msg162016
2012-05-31 15:30:06brett.cannonsetmessages: + msg162007
2012-05-30 18:13:51Ronan.Lamysetmessages: + msg161964
2012-05-30 14:31:44brett.cannonsetmessages: + msg161958
2012-05-29 20:37:42Ronan.Lamysetmessages: + msg161914
2012-05-29 14:38:14brett.cannonsetmessages: + msg161889
2012-05-29 01:25:20Ronan.Lamysetstatus: open -> closed
resolution: wont fix
messages: + msg161826
2012-05-29 01:23:53Ronan.Lamysetstatus: closed -> open
resolution: wont fix -> (no value)
messages: + msg161825
2012-05-28 21:31:27Arfreversetnosy: + Arfrever
2012-05-28 21:28:18brett.cannonsetstatus: open -> closed
resolution: wont fix
messages: + msg161809
2012-05-28 19:32:41Aaron.Meurersetnosy: + Aaron.Meurer
2012-05-28 19:15:34r.david.murraysetnosy: + brett.cannon
2012-05-28 18:52:19Ronan.Lamycreate