classification
Title: quote bad module name in ImportError-like messages
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: brett.cannon, eric.araujo, eric.snow, ezio.melotti, georg.brandl, haypo, ncoghlan, python-dev, terry.reedy
Priority: normal Keywords: easy, needs review, patch

Created on 2010-05-18 17:03 by terry.reedy, last changed 2013-02-01 22:56 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
import-repr.diff eric.araujo, 2010-12-07 20:47
import-repr-lib.diff eric.araujo, 2011-09-02 16:09 review
Messages (31)
msg105988 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-05-18 17:03
ImportError messages should quote the name it cannot import since the actual problem may be whitespace in the name that is currently invisible in the message.  In other words, display

ImportError: no module named 'bad name\r'

instead of

ImportError: no module named bad name

This defect lead to the current python-list thread
  pickle unable to load collection

Peter Otten figured out that it was unable to load 'collections\r' rather than 'collections', which he demonstrated with

>>> >>> try: pickle.loads(garbled_data)
... except ImportError as e:
...     e
...
ImportError('No module named collections\r',)

The OP used 2.6, I tested 3.1, hence presume, after searching tracker issues, that this applies to 2.7 and 3.2 as well.

I marked this as a bug since the current message is wrong and misleading. I suspect the same may be true of a few other error messages, but I cannot think of any at the moment.
msg105994 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2010-05-18 17:47
I guess it's a question of readability. Does::

  ImportError: No module named mod

read better than::

  ImportError: No module named 'mod'

In my eyes it doesn't by much, so switching to using repr() seems reasonable.

This can't be changed in released versions of Python as that could break someone's doctests. So making this only for 2.7 and 3.2.
msg106026 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-05-19 02:02
A refinement would be to only quote when there is whitespace in the name, but I do not know how well that works with unicode versus ascii.
msg106036 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2010-05-19 05:28
It wouldn't matter (at least in Python 3) as str is unicode-aware. It's more about whether it's worth special-casing the output. I say no and just go with using the repr.
msg121415 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-18 02:08
A question about process: Should every import bug be reported against core and library, so that importlib gets patched too?
msg121419 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2010-11-18 02:16
I think everyone knows that importlib is there and might need to be patched. Plus I run importlib against the entire test suite already on occasion so changes which has a proper test will eventually get caught.

So just file it against core.
msg121426 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-18 02:40
Thanks for the reply.

Funny thing, grep revealed a small inconsistency in this error message:

Python/pythonrun.c:415:     * XXX     Exception exceptions.ImportError: 'No module named sha'
Python/import.c:1821:                     "No module named %.200s", name);
Python/import.c:2722:                     "No module named %.200s", name);
Lib/test/test_multiprocessing.py:27:# message: "No module named _multiprocessing". _multiprocessing is not compiled
Lib/test/test_concurrent_futures.py:8:# message: "No module named _multiprocessing". _multiprocessing is not compiled
Lib/test/test_pydoc.py:194:badimport_pattern = "problem in %s - ImportError: No module named %s"
Lib/runpy.py:104:        raise ImportError("No module named %s" % mod_name)
Lib/xml/etree/ElementTree.py:1492:                    "No module named expat; use SimpleXMLTreeBuilder instead"
Lib/modulefinder.py:189:        self.msgout(4, "raise ImportError: No module named", qname)
Lib/modulefinder.py:190:        raise ImportError("No module named " + qname)
Lib/modulefinder.py:202:                self.msgout(4, "raise ImportError: No module named", mname)
Lib/modulefinder.py:203:                raise ImportError("No module named " + mname)
Lib/modulefinder.py:219:                    raise ImportError("No module named " + subname)
Lib/importlib/_bootstrap.py:811:                raise ImportError("no module named {}; "
Lib/importlib/_bootstrap.py:820:            raise ImportError("No module named {0}".format(name))
Lib/unittest/test/test_loader.py:242:            self.assertEqual(str(e), "No module named sdasfasfasdf")
Lib/unittest/test/test_loader.py:622:            self.assertEqual(str(e), "No module named sdasfasfasdf")

Fixing the Python modules is easy, but I don’t know how to change PyErr_Format (or how to write C, for that matter :)  Do you make offers like Alexandre, who has proposed to write the C part if someone provides a diff for the Python version?
msg121427 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-18 02:42
I should have included only the one line that’s different:
Lib/importlib/_bootstrap.py:811:                raise ImportError("no
(lower-case n)
msg121430 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2010-11-18 02:56
PyErr_Format doesn't need to change, just it's argument. A call to PyObject_Repr() (w/ proper error checking) should be all that is needed.

And no, I don't make any "you do the Python, I'll do the C" deals because the Python part is the fun part. =)
msg121432 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-18 03:13
> A call to PyObject_Repr() (w/ proper error checking) should be all
> that is needed.
Sadly out of reach for me.

> And no, I don't make any "you do the Python, I'll do the C" deals
> because the Python part is the fun part. =)
I understand :)

FTR, case inconsistency fixed by Brett in r86507.
msg121853 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-21 03:28
Just for fun, I tried using my tiny understanding of C to write a patch. I am attaching my current result, which passes the test suite except for test_unittest:

FAIL: test_loadTestsFromName__unknown_module_name (unittest.test.test_loader.Test_TestLoader)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/wok/code/hg/cpython/3.2/fix-import-repr/Lib/unittest/test/test_loader.py", line 240, in test_loadTestsFromName__unknown_module_name
    loader.loadTestsFromName('sdasfasfasdf')
ImportError: No module named 0I�

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/wok/code/hg/cpython/3.2/fix-import-repr/Lib/unittest/test/test_loader.py", line 242, in test_loadTestsFromName__unknown_module_name
    self.assertEqual(str(e), "No module named 'sdasfasfasdf'")
AssertionError: 'No module named 0I�\x01' != "No module named 'sdasfasfasdf'"
- No module named 0I�
+ No module named 'sdasfasfasdf'


======================================================================
FAIL: test_loadTestsFromNames__unknown_module_name (unittest.test.test_loader.Test_TestLoader)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Lib/unittest/test/test_loader.py", line 620, in test_loadTestsFromNames__unknown_module_name
    loader.loadTestsFromNames(['sdasfasfasdf'])
ImportError: No module named HB�

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "Lib/unittest/test/test_loader.py", line 622, in test_loadTestsFromNames__unknown_module_name
    self.assertEqual(str(e), "No module named 'sdasfasfasdf'")
AssertionError: 'No module named HB�\x01' != "No module named 'sdasfasfasdf'"
- No module named HB�
+ No module named 'sdasfasfasdf'

test_imp{,ort,ortlib} pass, though.  Maybe someone will take my patch and fix the PyObject_Repr call and error checking.  It was a fun bit of experimenting with help from kind people on IRC :)
msg123579 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-07 20:47
I fixed my patch, thanks to a recent commit that showed me an example of PyErr_Format :)  All tests now pass.
msg123583 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2010-12-07 22:31
The patch looks same to me as far as I can judge. I would have used .format instead of %, but you wrote it ;-).

Seeing how many of our tests had to be patched convinced me that we should treat this like a feature request and only apply to 3.2.
msg123629 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-08 18:33
> The patch looks same to me as far as I can judge.
Thanks.  Can you apply the patch on your Windows machine and run the test suite?

> I would have used .format instead of %, but you wrote it ;-).
I would have too, were I writing Python :)  Here I tried to do the simplest thing that could work.

> Seeing how many of our tests had to be patched convinced me that we
> should treat this like a feature request and only apply to 3.2.
Yes, as an incompatible behaviour change I cannot go into released versions, as Brett stated in msg105994.
msg124140 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-16 17:29
Does someone have time to review?  I think this would be a good change for 3.2.

Georg, please lower the priority if you think this can wait for 3.3.
msg124212 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-17 15:40
The change would be fine with me.  What happens with the PyUnicode_FromString() usage in the patch if the string cannot be decoded?  That should not lead to a UnicodeError being raised.

Also, the __main__ changes look gratuitous to me.
msg124336 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-19 10:04
Deferring, this is not a bug.
msg124470 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-12-22 00:20
I set LANG and LC_ALL to C and tried to import a module with a non-ASCII name:
    $ ./python -m échec™♥
    python: No module named '\udcc3\udca9chec\udce2\udc84\udca2\udce2\udc99\udca5'
Is that a good enough test?

I guess the “__main__ changes” you’re talking about are the addition of single quotes around __main__ in some error messages; that was only for the sake of consistency.
msg124496 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2010-12-22 09:57
I suppose it's not a good test, since your non-ascii name presumably was encoded in UTF-8, which is the encoding that PyUnicode_FromString uses.
msg126325 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2011-01-15 13:11
Won't make it into 3.2.
msg126757 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-01-21 17:10
My patch for #3080 uses repr() to format module name in all error messages.
msg130541 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-03-11 02:32
Victor: Should I close this bug as superseded by yours?
msg131517 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-03-20 17:17
Victor changed the code to use repr in some modules but not all in c4361bab6914.
msg131936 - (view) Author: Roundup Robot (python-dev) Date: 2011-03-23 23:07
New changeset 9f9b7b656761 by Brett Cannon in branch 'default':
Have importlib use the repr of a module name in error messages.
http://hg.python.org/cpython/rev/9f9b7b656761
msg131941 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-03-23 23:18
I patched import.c to use repr() instead of str() (%R instead of %U) to format module names because they might contain surrogate characters. Surrogate characters are not encodable to any encoding, except UTF-16 and UTF-32. And so print an exception to stdout may produce an UnicodeEncodeError. At the same time... exceptions are printed to stderr which uses the backslashreplace error handler, and so the message *can* be printed:

$ python3.1
>>> raise Exception('x\uDC80y')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
Exception: x\udc80y

So now I realized that my change was maybe useless.
msg143409 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-02 16:09
Victor changed ImportError some time ago for #3080, so my patch now only touches a few stdlib modules.
msg152997 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-02-09 22:48
With issue1559549 adding a 'name' argument, I'm going to push to have it gain a reasonable default __str__ if 'name' is set but nothing else. That patch also contains some helper functions which should simplify Eric's patch.
msg154750 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-03-02 08:30
> With issue1559549 adding a 'name' argument, I'm going to push to have it gain a reasonable default
> __str__ if 'name' is set but nothing else. That patch also contains some helper functions which
> should simplify Eric's patch.

Maybe you’re talking about my first patch?  The latest one does not touch C code and I don’t see why it depends on the other bug.
msg155109 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-03-07 19:56
It technically doesn't depend, but it potentially would make this moot. But stuff is going on at the language summit which is going to shift stuff around.
msg181119 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2013-02-01 22:05
Over time I have fixed this issue in various patches.
msg181129 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2013-02-01 22:56
Great!
History
Date User Action Args
2013-02-01 22:56:17eric.araujosetmessages: + msg181129
stage: patch review -> resolved
2013-02-01 22:05:16brett.cannonsetstatus: open -> closed
resolution: fixed
messages: + msg181119
2012-03-08 22:14:41ezio.melottisetnosy: + ezio.melotti
2012-03-07 19:56:48brett.cannonsetdependencies: - ImportError needs attributes for module and file name
messages: + msg155109
2012-03-02 08:30:23eric.araujosetmessages: + msg154750
2012-02-09 22:48:17brett.cannonsetdependencies: + ImportError needs attributes for module and file name
messages: + msg152997
2011-11-25 08:41:19eric.snowsetnosy: + eric.snow
2011-09-02 16:09:59eric.araujosetfiles: + import-repr-lib.diff

messages: + msg143409
components: + Library (Lib), - Interpreter Core
title: ImportError: quote bad module name in message -> quote bad module name in ImportError-like messages
2011-03-23 23:18:52hayposetassignee: haypo -> eric.araujo
messages: + msg131941
2011-03-23 23:07:23python-devsetnosy: + python-dev
messages: + msg131936
2011-03-23 22:50:01brett.cannonsetassignee: haypo
2011-03-20 17:17:54eric.araujosetnosy: brett.cannon, georg.brandl, terry.reedy, ncoghlan, haypo, eric.araujo
messages: + msg131517
2011-03-15 19:35:18ncoghlansetnosy: + ncoghlan
2011-03-11 02:32:44eric.araujosetpriority: high -> normal
nosy: brett.cannon, georg.brandl, terry.reedy, haypo, eric.araujo
messages: + msg130541
2011-01-21 17:10:00hayposetnosy: + haypo
messages: + msg126757
2011-01-15 13:11:47georg.brandlsetpriority: deferred blocker -> high

messages: + msg126325
versions: + Python 3.3, - Python 3.2
2010-12-22 09:57:37georg.brandlsetmessages: + msg124496
2010-12-22 00:20:28eric.araujosetmessages: + msg124470
2010-12-19 10:04:25georg.brandlsetpriority: release blocker -> deferred blocker

messages: + msg124336
2010-12-17 15:40:13georg.brandlsetmessages: + msg124212
2010-12-16 17:29:10eric.araujosetpriority: low -> release blocker
nosy: + georg.brandl
messages: + msg124140

2010-12-08 18:33:54eric.araujosetmessages: + msg123629
2010-12-07 22:31:12terry.reedysetmessages: + msg123583
2010-12-07 20:47:52eric.araujosetfiles: - import-repr.diff
2010-12-07 20:47:44eric.araujosetkeywords: + needs review
files: + import-repr.diff
messages: + msg123579

stage: needs patch -> patch review
2010-11-21 03:28:15eric.araujosetfiles: + import-repr.diff
keywords: + patch
messages: + msg121853
2010-11-18 03:13:46eric.araujosetmessages: + msg121432
2010-11-18 02:56:13brett.cannonsetmessages: + msg121430
2010-11-18 02:42:13eric.araujosetmessages: + msg121427
2010-11-18 02:40:44eric.araujosetmessages: + msg121426
2010-11-18 02:16:28brett.cannonsetmessages: + msg121419
2010-11-18 02:08:49eric.araujosetnosy: + eric.araujo

messages: + msg121415
versions: - Python 2.7
2010-05-19 05:28:49brett.cannonsetmessages: + msg106036
2010-05-19 02:02:27terry.reedysetmessages: + msg106026
2010-05-19 02:02:12terry.reedysetmessages: - msg106025
2010-05-19 02:01:25terry.reedysetmessages: + msg106025
2010-05-18 17:48:02brett.cannonsetkeywords: + easy
2010-05-18 17:47:49brett.cannonsetpriority: normal -> low
versions: - Python 2.6, Python 3.1
nosy: + brett.cannon

messages: + msg105994

stage: needs patch
2010-05-18 17:03:18terry.reedycreate