-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
using custom objects as modules: AttributeErrors new in 3.5 #68680
Comments
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 bpo-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. |
What's a sensible approach to ameliorate the problem? Gracefully muddle through without a __name__ on the imported object? |
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 |
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...) |
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. |
This is While I want this fixed, I'm not going to hold up beta 3 for it. |
Here is a patch that adds a test, fixes the issue, and for good measure stops assuming that __name__ will always be a string. |
Larry, does this warrant going into 3.5.0? |
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). |
patch LGTM. Presumably the divergence between importlib (in _handle_fromlist) and import.c was strictly accidental (i.e. lack of test coverage). |
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. |
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. |
Yes, Eric and Armin are both qualified reviewers in my book. You have my blessing to send a pull request. Thanks, everybody! |
My Bitbucket repo is now public. |
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. |
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? |
Yep. This time I have foisted nearly all the work, including the forward-merging, onto y'all. *sits back, sips iced coffee* |
New changeset cf3a62a8d786 by Brett Cannon in branch '3.5': New changeset bbe6b215df5d by Brett Cannon in branch '3.5': New changeset b0a6bba16b9c by Brett Cannon in branch 'default': |
I have merged my 3.5.0 patch into 3.5 and default, so that should fix the issue Armin and CFFI was having. |
New changeset 5a9ac801f9b4 by larry in branch '3.5': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: