classification
Title: using custom objects as modules: AttributeErrors new in 3.5
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brett.cannon Nosy List: arigo, brett.cannon, eric.snow, larry, ncoghlan, python-dev, terry.reedy
Priority: release blocker Keywords: 3.4regression, patch

Created on 2015-06-23 17:40 by arigo, last changed 2015-08-24 20:01 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
test_case.py arigo, 2015-06-23 17:45
issue24492.diff brett.cannon, 2015-08-09 23:40 review
Messages (20)
msg245696 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-06-23 17:40
A regression in 3.5: if we use custom objects as modules (like some projects do), then these custom objects may not have an attribute called "__name__".  There is new code in 3.5 added for issue #17636 which tries sometimes to read the "__name__" of a module when executing an import statement, and if this leads to an AttributeError, it will get propagated.

I imagine this could break real code using "try: except ImportError:".  It _does_ break the tests of the "cffi" project, which try to check that some import of a nonexisting name correctly gives ImportError.  Now it gives AttributeError("__name__") instead.
msg245699 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-06-23 19:52
What's a sensible approach to ameliorate the problem?  Gracefully muddle through without a __name__ on the imported object?
msg245725 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-06-24 08:29
I'd guess so: if the PyObject_GetAttrId() fails, then ignore the rest of the code that was added in https://hg.python.org/cpython/rev/fded07a2d616 and jump straight to ``PyErr_Format(PyExc_ImportError, ...)``.
msg245726 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-06-24 08:35
Also, if we want to be paranoid, the _PyObject_GetAttrId() can return anything, not necessarily a string object.  This would make the following PyUnicode_FromFormat() fail.  So maybe you also want to overwrite failures in PyUnicode_FromFormat() with the final ImportError.  (It actually looks like a good--if convoluted--use case for exception chaining at the C level...)
msg245869 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2015-06-26 21:05
I view having a string __name__ attribute as part of quacking like a module (or class or function).  Hence pseudonames like '<lambda>' (lambda expression) or <pyshell#n> (the module name for interactive input, where n is the line number).  Is there a good reason for custom 'module' objects to not have a name?  If so, the 'graceful muddle' precedent is to give them a pseudoname, such as <module>, perhaps with a prefix or suffix.
msg246112 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-07-03 00:52
This is
  a) marked as release blocker, and
  b) is assigned to nobody.
This is not tenable.

While I want this fixed, I'm not going to hold up beta 3 for it.
msg248345 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-09 23:39
Here is a patch that adds a test, fixes the issue, and for good measure stops assuming that __name__ will always be a string.
msg248361 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-10 16:50
Larry, does this warrant going into 3.5.0?
msg248368 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-08-10 20:44
I would like the fix in 3.5.  However, I'm not qualified to review the code.  Can you get a qualified reviewer in to look over the code?

Once someone suitable has reviewed it, I'll accept a pull request (pasted in here naturally).
msg248369 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2015-08-10 21:08
patch LGTM.  Presumably the divergence between importlib (in _handle_fromlist) and import.c was strictly accidental (i.e. lack of test coverage).
msg248371 - (view) Author: Armin Rigo (arigo) * (Python committer) Date: 2015-08-10 21:24
Patch LGTM too.  Optionally a test is needed for each of the other cases in it too, but please don't cause that comment to stop it from getting in.
msg248373 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-10 21:36
Thanks for the reviews, Eric and Armin. I will get the patch in 3.5.0, 3.5.1, and 3.6 sometime this week and then open another issue for adding more tests for the other branches in the code.
msg248377 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-08-10 22:51
Yes, Eric and Armin are both qualified reviewers in my book.  You have my blessing to send a pull request.

Thanks, everybody!
msg248384 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-08-11 00:29
My Bitbucket repo is now public.

https://bitbucket.org/larry/cpython350
msg248436 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-12 01:10
Created https://bitbucket.org/larry/cpython350/pull-requests/2/issue-24492-make-sure-that-from-import/diff without the PyUnicode_FromFormat() change as I realized that was conflating two changes in one patch and it wasn't even being tested (verified all tests still pass and a quick check in the interpreter shows ImportError is still raised).

Larry, let me know when you have accepted the PR and I will commit to 3.5 and default on hg.python.org.

I also created http://bugs.python.org/issue24846 to track adding more tests for the code.
msg248541 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-13 21:45
I noticed you accepted the PR on Bitbucket, Larry. Should I consider your part done and I can now pull the commit into the 3.5 and default branches on hg.python.org?
msg248551 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2015-08-14 01:44
Yep.  This time I have foisted nearly all the work, including the forward-merging, onto y'all.

*sits back, sips iced coffee*
msg248598 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-08-14 18:10
New changeset cf3a62a8d786 by Brett Cannon in branch '3.5':
Issue #24492: make sure that ``from ... import ...` raises an
https://hg.python.org/cpython/rev/cf3a62a8d786

New changeset bbe6b215df5d by Brett Cannon in branch '3.5':
Merge from 3.5.0 for issue #24492
https://hg.python.org/cpython/rev/bbe6b215df5d

New changeset b0a6bba16b9c by Brett Cannon in branch 'default':
Merge from 3.5 for issue #24492
https://hg.python.org/cpython/rev/b0a6bba16b9c
msg248599 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2015-08-14 18:11
I have merged my 3.5.0 patch into 3.5 and default, so that should fix the issue Armin and CFFI was having.
msg249072 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-08-24 20:01
New changeset 5a9ac801f9b4 by larry in branch '3.5':
Merged in brettcannon/cpython350/3.5 (pull request #2)
https://hg.python.org/cpython/rev/5a9ac801f9b4
History
Date User Action Args
2015-08-24 20:01:24python-devsetmessages: + msg249072
2015-08-14 18:11:34brett.cannonsetstatus: open -> closed
messages: + msg248599

assignee: larry -> brett.cannon
resolution: fixed
stage: commit review -> resolved
2015-08-14 18:10:29python-devsetnosy: + python-dev
messages: + msg248598
2015-08-14 01:44:11larrysetmessages: + msg248551
2015-08-13 21:45:26brett.cannonsetmessages: + msg248541
2015-08-12 01:10:28brett.cannonsetassignee: brett.cannon -> larry
messages: + msg248436
2015-08-12 01:10:00brett.cannonlinkissue24846 dependencies
2015-08-11 00:29:18larrysetmessages: + msg248384
2015-08-10 22:51:34larrysetmessages: + msg248377
2015-08-10 21:36:34brett.cannonsetpriority: deferred blocker -> release blocker

messages: + msg248373
2015-08-10 21:24:55arigosetmessages: + msg248371
2015-08-10 21:08:36eric.snowsetmessages: + msg248369
stage: patch review -> commit review
2015-08-10 20:44:36larrysetassignee: larry -> brett.cannon
2015-08-10 20:44:09larrysetmessages: + msg248368
2015-08-10 16:50:29brett.cannonsetassignee: brett.cannon -> larry
messages: + msg248361
2015-08-09 23:40:00brett.cannonsetfiles: + issue24492.diff
keywords: + patch
messages: + msg248345

stage: needs patch -> patch review
2015-08-09 18:47:40brett.cannonsetassignee: brett.cannon
versions: + Python 3.6
2015-07-21 07:05:36ethan.furmansetnosy: - ethan.furman
2015-07-03 00:52:54larrysetpriority: release blocker -> deferred blocker

messages: + msg246112
2015-06-26 21:07:54terry.reedysetstage: needs patch
2015-06-26 21:05:19terry.reedysetnosy: + terry.reedy
messages: + msg245869
2015-06-24 08:35:17arigosetmessages: + msg245726
2015-06-24 08:29:24arigosetmessages: + msg245725
2015-06-23 19:52:50larrysetmessages: + msg245699
2015-06-23 18:16:29r.david.murraysetpriority: normal -> release blocker
2015-06-23 17:45:30arigosetfiles: + test_case.py
2015-06-23 17:43:59ethan.furmansetnosy: + brett.cannon, ncoghlan, ethan.furman, eric.snow
2015-06-23 17:40:13arigocreate