classification
Title: __loader__ = None should be fine
Type: behavior Stage: resolved
Components: Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: 17099 17117 Superseder:
Assigned To: brett.cannon Nosy List: brett.cannon, eric.snow, ezio.melotti, gkcn, pitrou, python-dev
Priority: normal Keywords:

Created on 2013-02-03 16:47 by brett.cannon, last changed 2013-05-04 21:37 by python-dev. This issue is now closed.

Messages (11)
msg181279 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-03 16:47
There is no reason __loader__ can't be set to None like __package__. That means having xml.parsers.expat.(model|errors) set the attribute to None by default (and thus removing the exemption for those modules from test_importlib.test_api.StartupTests), updating the decorators in importlib.util to set __loader__ when it is None, and make importlib.find_loader() treat None the same as if the attribute isn't set (e.g. unifying the raising of ValueError in both cases).

This will bring __loader__ back in alignment with __package__.

In all honesty I would like to tweak imp.new_module()/PyModule_Create() to set both __package__ and __loader__ to None by default, but I don't know how much code out there relies on the absence of these attributes and not on them being set to None (although fixing all of that code is simply transitioning from hasattr(module, '__loader__') to getattr(module, '__loader__', None)). Luckily PEP 302 was updated a couple versions back to state the loaders **must** set these attributes, so hopefully as time goes on this will be less of a worry.
msg181283 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-03 17:03
Screw it, forget the "like" and make that a "will happen". A What's New entry telling people to update their code to use a getattr() with a None default value for transitioning should be enough (and a single line change for the few people who would care about this).

This will also require an updating of the importlib docs, language reference, and PEP 302 to say the language reference and importlib docs now supersede the PEP. For both attributes it should be stated that a value of None means "I don't know what the values should be", but that loaders should continue to be required to set them.
msg181293 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-02-03 20:02
Mmmh, what is the point of forcing people to adapt their module-like objects so that they have a __loader__ entry?
Couldn't importlib just interpret the absence of __loader__ as a None?
msg181301 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-03 21:24
It will interpret its absence as None, but I would rather get the very common case of actual module instances just setting None automatically. Not having it set when None is already an accepted default value for __package__ seems inconsistent; either the lack of attribute should signify that the value is unknown for both values, or having None should, but not both. I'm advocating for the latter as it has an easier compatibility path. Plus I'm not worrying about random objects people stash into sys.modules, just real modules created from imp.new_module()/PyModule_Create(); if you muck with sys.modules you are on your own.
msg181359 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-04 17:53
> In all honesty I would like to tweak imp.new_module()/PyModule_Create()...

+1
msg181360 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2013-02-04 17:56
> [document that] the language reference and importlib docs now supersede the PEP

Agreed.  PEP 302 is even crustier now than it was a year ago and Barry's new import page in the language reference obviates the need for 302 as the de facto spec.
msg188381 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-04 17:57
New changeset e39a8f8ceb9f by Brett Cannon in branch 'default':
#17115,17116: Have modules initialize the __package__ and __loader__
http://hg.python.org/cpython/rev/e39a8f8ceb9f
msg188389 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-05-04 18:37
Some buildbots are failing after this:
http://buildbot.python.org/all/builders/x86%20Ubuntu%20Shared%203.x/builds/7840
http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.0%203.x/builds/4536

FAIL: test_everyone_has___loader__ (test.test_importlib.test_api.StartupTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_importlib/test_api.py", line 201, in test_everyone_has___loader__
    '{!r} lacks a __loader__ attribute'.format(name))
AssertionError: False is not true : 'test.pydoc_mod' lacks a __loader__ attribute
msg188396 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-05-04 18:48
It's because test_pydoc is actively deleting the __loader__ of a test module. I'll have a look to figure out why it's doing that.
msg188410 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-04 21:29
New changeset bb023c3426bc by Brett Cannon in branch 'default':
#17115: Remove what appears to be a useless chunk of code which broke
http://hg.python.org/cpython/rev/bb023c3426bc
msg188411 - (view) Author: Roundup Robot (python-dev) Date: 2013-05-04 21:37
New changeset 97b7bd87c44c by Brett Cannon in branch 'default':
#17115: I hate you MS for not supporting C99.
http://hg.python.org/cpython/rev/97b7bd87c44c
History
Date User Action Args
2013-05-04 21:37:17python-devsetmessages: + msg188411
2013-05-04 21:30:05brett.cannonsetstatus: open -> closed
resolution: fixed
2013-05-04 21:29:43python-devsetmessages: + msg188410
2013-05-04 18:48:21brett.cannonsetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg188396
2013-05-04 18:37:36ezio.melottisetnosy: + ezio.melotti
messages: + msg188389
2013-05-04 17:58:11brett.cannonsetstatus: open -> closed
resolution: fixed
stage: test needed -> resolved
2013-05-04 17:57:09python-devsetnosy: + python-dev
messages: + msg188381
2013-04-28 02:52:52brett.cannonsetdependencies: - xml.parsers.expat.(errors|model) don't set the __loader__ attribute
2013-02-23 17:27:37gkcnsetnosy: + gkcn
2013-02-08 14:01:12brett.cannonlinkissue17116 dependencies
2013-02-04 17:56:55eric.snowsetmessages: + msg181360
2013-02-04 17:53:09eric.snowsetnosy: + eric.snow
messages: + msg181359
2013-02-03 21:24:14brett.cannonsetmessages: + msg181301
2013-02-03 20:02:29pitrousetnosy: + pitrou
messages: + msg181293
2013-02-03 17:03:50brett.cannonsetmessages: + msg181283
2013-02-03 16:49:43brett.cannonsetdependencies: + Update importlib.util.module_for_loader/set_loader to set when __loader__ = None
2013-02-03 16:48:10brett.cannonsetdependencies: + Raise ValueError when __loader__ not defined for importlib.find_loader()
2013-02-03 16:47:46brett.cannonsetdependencies: + xml.parsers.expat.(errors|model) don't set the __loader__ attribute
2013-02-03 16:47:26brett.cannoncreate