msg199847 - (view) |
Author: Alex Gaynor (alex) *  |
Date: 2013-10-14 07:29 |
Use case:
Alexanders-MacBook-Pro:pypy alex_gaynor$ python3.3
Python 3.3.2 (default, May 29 2013, 14:03:57)
[GCC 4.2.1 Compatible Apple LLVM 4.2 (clang-425.0.28)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import datetime.datetime
Traceback (most recent call last):
File "<frozen importlib._bootstrap>", line 1521, in _find_and_load_unlocked
AttributeError: 'module' object has no attribute '__path__'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ImportError: No module named 'datetime.datetime'; datetime is not a package
>>>
Showing the user the original "module has no attribute __path__" error is just confusing, we should hide it.
|
msg213368 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2014-03-13 09:17 |
Fix is simple:
--- a/Lib/importlib/_bootstrap.py
+++ b/Lib/importlib/_bootstrap.py
@@ -1519,7 +1519,7 @@ def _find_and_load_unlocked(name, import
path = parent_module.__path__
except AttributeError:
msg = (_ERR_MSG + '; {} is not a package').format(name, parent)
- raise ImportError(msg, name=name)
+ raise ImportError(msg, name=name) from None
Manual testing confirms the fix. Is a unit test needed? Should this wait for 3.5?
|
msg213369 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2014-03-13 09:18 |
> --- a/Lib/importlib/_bootstrap.py
> +++ b/Lib/importlib/_bootstrap.py
Eric, please attach the patch as a file to the issue, so it can be easily reviewed on Rietveld.
|
msg213370 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2014-03-13 09:20 |
I figured it would take less time for someone to go to the file and make the edit than download and apply a patch file, but as you wish :)
|
msg213372 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2014-03-13 09:25 |
"I figured it would take less time for someone to go to the file and make the edit than download and apply a patch file, but as you wish :)"
It's not to apply the patch, but to review it. The Rietveld tool helps to review a patch online, it displays more context (lines before/after) ;-)
|
msg213394 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2014-03-13 13:09 |
Is it confusing though? If you only had the latter exception all you would know is Python doesn't consider datetime a package but you wouldn't know why that is unless you knew the exact definition (the __path__ attribute exists). Having the former exception helps make that a bit more obvious if you didn't already know it.
|
msg213485 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2014-03-13 22:24 |
> If you only had the latter exception all you would know is Python
> doesn't consider datetime a package but you wouldn't know why that is
Well, I’d be satisfied with that. Looking at the doc or importing just datetime would let me know it’s a module, not a package.
> unless you knew the exact definition (the __path__ attribute exists).
To me that’s an implementation detail. The real definition in my mental model is that modules are files and packages are directories. __path__ is advanced stuff.
|
msg213487 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2014-03-13 22:30 |
That's only true if you are dealing with files and directories though. The import system can deal with much more than that, in which case the absence __path__ could be an important bit of debugging information.
|
msg213490 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2014-03-13 22:33 |
Actually, it could be important even if you are dealing with files and directories (or I'm confused and __path__ is always about same :), since you could be dealing with an alternate loader.
Our policy is not to hide the chained traceback unless *all* of the information in the chain has been expressed in the replacement traceback error message.
|
msg213518 - (view) |
Author: Éric Araujo (eric.araujo) *  |
Date: 2014-03-14 06:32 |
That makes a lot of sense. I guess this is Brett’s call.
|
msg213536 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2014-03-14 11:52 |
> If you only had the latter exception all you would know is Python
> doesn't consider datetime a package but you wouldn't know why that is
> unless you knew the exact definition (the __path__ attribute exists).
> Having the former exception helps make that a bit more obvious if you
> didn't already know it.
I don't think someone who doesn't know about __path__ would be very enlightened by the error message. Also, I don't think error messages have a role in teaching about the implementation details of features, so I'd vote for removing the __context__ here.
|
msg213542 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2014-03-14 13:01 |
Antoine: down that path lies Microsoft's "An error has occurred" error messages. The point of the extra information is not to inform the end user, it is to make it possible for an expert to solve the problem, and for it to be findable in a web search.
Now, whether or not there are enough different things that the second error could be chained off of to make that worthwhile in this case is something Brett or Eric will have to answer. My guess is that there *potentially* are, even though this one is the most common, but that's just a guess since I haven't studied importlib.
|
msg213544 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2014-03-14 13:16 |
> Antoine: down that path lies Microsoft's "An error has occurred" error
> messages. The point of the extra information is not to inform the end
> user, it is to make it possible for an expert to solve the problem,
> and for it to be findable in a web search.
I don't know how that's related. Here the error is trying to import from
a package while the module isn't a package. The solution is either to
change the import to something else, or to turn the non-package into a
package. None of these involve doing anything explicitly with __path__,
so the original error is a distraction.
|
msg363545 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2020-03-06 20:33 |
I'm going to make a call and say the chained exception should stay. If people want to start the discussion again they can.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:52 | admin | set | github: 63456 |
2020-03-06 20:33:53 | brett.cannon | set | status: open -> closed resolution: rejected messages:
+ msg363545
stage: patch review -> resolved |
2014-03-14 13:16:41 | pitrou | set | messages:
+ msg213544 |
2014-03-14 13:01:25 | r.david.murray | set | messages:
+ msg213542 |
2014-03-14 11:52:37 | pitrou | set | nosy:
+ pitrou messages:
+ msg213536
|
2014-03-14 06:32:26 | eric.araujo | set | messages:
+ msg213518 |
2014-03-13 22:33:21 | r.david.murray | set | messages:
+ msg213490 |
2014-03-13 22:30:15 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg213487
|
2014-03-13 22:24:34 | eric.araujo | set | messages:
+ msg213485 |
2014-03-13 13:09:12 | brett.cannon | set | messages:
+ msg213394 versions:
+ Python 3.5, - Python 3.4 |
2014-03-13 09:25:52 | vstinner | set | messages:
+ msg213372 |
2014-03-13 09:20:53 | eric.araujo | set | files:
+ fix-import-not-package-tb.diff
messages:
+ msg213370 |
2014-03-13 09:18:49 | vstinner | set | nosy:
+ vstinner messages:
+ msg213369
|
2014-03-13 09:18:04 | eric.araujo | set | keywords:
+ patch, needs review stage: patch review |
2014-03-13 09:17:15 | eric.araujo | set | nosy:
+ eric.araujo messages:
+ msg213368
|
2013-10-14 09:02:45 | berker.peksag | set | nosy:
+ brett.cannon, eric.snow
versions:
+ Python 3.4 |
2013-10-14 07:29:15 | alex | create | |