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.

classification
Title: Sub-optimal error message when importing a non-package
Type: Stage: resolved
Components: Versions: Python 3.5
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: alex, brett.cannon, eric.araujo, eric.snow, pitrou, r.david.murray, vstinner
Priority: normal Keywords: needs review, patch

Created on 2013-10-14 07:29 by alex, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
fix-import-not-package-tb.diff eric.araujo, 2014-03-13 09:20 review
Messages (14)
msg199847 - (view) Author: Alex Gaynor (alex) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2022-04-11 14:57:52adminsetgithub: 63456
2020-03-06 20:33:53brett.cannonsetstatus: open -> closed
resolution: rejected
messages: + msg363545

stage: patch review -> resolved
2014-03-14 13:16:41pitrousetmessages: + msg213544
2014-03-14 13:01:25r.david.murraysetmessages: + msg213542
2014-03-14 11:52:37pitrousetnosy: + pitrou
messages: + msg213536
2014-03-14 06:32:26eric.araujosetmessages: + msg213518
2014-03-13 22:33:21r.david.murraysetmessages: + msg213490
2014-03-13 22:30:15r.david.murraysetnosy: + r.david.murray
messages: + msg213487
2014-03-13 22:24:34eric.araujosetmessages: + msg213485
2014-03-13 13:09:12brett.cannonsetmessages: + msg213394
versions: + Python 3.5, - Python 3.4
2014-03-13 09:25:52vstinnersetmessages: + msg213372
2014-03-13 09:20:53eric.araujosetfiles: + fix-import-not-package-tb.diff

messages: + msg213370
2014-03-13 09:18:49vstinnersetnosy: + vstinner
messages: + msg213369
2014-03-13 09:18:04eric.araujosetkeywords: + patch, needs review
stage: patch review
2014-03-13 09:17:15eric.araujosetnosy: + eric.araujo
messages: + msg213368
2013-10-14 09:02:45berker.peksagsetnosy: + brett.cannon, eric.snow

versions: + Python 3.4
2013-10-14 07:29:15alexcreate