Skip to content

quote bad module name in ImportError-like messages #53000

Closed

Description

@terryjreedy
BPO 8754
Nosy @brettcannon, @birkenfeld, @terryjreedy, @ncoghlan, @vstinner, @ezio-melotti, @merwok, @ericsnowcurrently
Files
  • import-repr.diff
  • import-repr-lib.diff
  • 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:

    assignee = 'https://github.com/merwok'
    closed_at = <Date 2013-02-01.22:05:16.967>
    created_at = <Date 2010-05-18.17:03:18.189>
    labels = ['easy', 'type-bug', 'library']
    title = 'quote bad module name in ImportError-like messages'
    updated_at = <Date 2013-02-01.22:56:17.712>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2013-02-01.22:56:17.712>
    actor = 'eric.araujo'
    assignee = 'eric.araujo'
    closed = True
    closed_date = <Date 2013-02-01.22:05:16.967>
    closer = 'brett.cannon'
    components = ['Library (Lib)']
    creation = <Date 2010-05-18.17:03:18.189>
    creator = 'terry.reedy'
    dependencies = []
    files = ['19971', '23088']
    hgrepos = []
    issue_num = 8754
    keywords = ['patch', 'easy', 'needs review']
    message_count = 31.0
    messages = ['105988', '105994', '106026', '106036', '121415', '121419', '121426', '121427', '121430', '121432', '121853', '123579', '123583', '123629', '124140', '124212', '124336', '124470', '124496', '126325', '126757', '130541', '131517', '131936', '131941', '143409', '152997', '154750', '155109', '181119', '181129']
    nosy_count = 9.0
    nosy_names = ['brett.cannon', 'georg.brandl', 'terry.reedy', 'ncoghlan', 'vstinner', 'ezio.melotti', 'eric.araujo', 'python-dev', 'eric.snow']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue8754'
    versions = ['Python 3.3']

    Activity

    1. terryjreedy commented on May 18, 2010

      @terryjreedy
      MemberAuthor

      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.

    2. added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      type-bugAn unexpected behavior, bug, or error
      on May 18, 2010
    3. brettcannon commented on May 18, 2010

      @brettcannon
      Member

      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.

    4. terryjreedy commented on May 19, 2010

      @terryjreedy
      MemberAuthor

      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.

    5. brettcannon commented on May 19, 2010

      @brettcannon
      Member

      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.

    6. merwok commented on Nov 18, 2010

      @merwok
      Member

      A question about process: Should every import bug be reported against core and library, so that importlib gets patched too?

    7. brettcannon commented on Nov 18, 2010

      @brettcannon
      Member

      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.

    8. merwok commented on Nov 18, 2010

      @merwok
      Member

      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?

    9. merwok commented on Nov 18, 2010

      @merwok
      Member

      I should have included only the one line that’s different:
      Lib/importlib/_bootstrap.py:811: raise ImportError("no
      (lower-case n)

    10. brettcannon commented on Nov 18, 2010

      @brettcannon
      Member

      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. =)

    11. merwok commented on Nov 18, 2010

      @merwok
      Member

      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.

    12. merwok commented on Nov 21, 2010

      @merwok
      Member

      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 :)

    13. merwok commented on Dec 7, 2010

      @merwok
      Member

      I fixed my patch, thanks to a recent commit that showed me an example of PyErr_Format :) All tests now pass.

    14. 16 remaining items

    15. python-dev commented on Mar 23, 2011

      python-devmannequin
      Mannequin

      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

    16. vstinner commented on Mar 23, 2011

      @vstinner
      Member

      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.

    17. assigned and unassigned on Mar 23, 2011
    18. merwok commented on Sep 2, 2011

      @merwok
      Member

      Victor changed ImportError some time ago for bpo-3080, so my patch now only touches a few stdlib modules.

    19. added
      stdlibStandard Library Python modules in the Lib/ directory
      and removed
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      on Sep 2, 2011
    20. changed the title [-]ImportError: quote bad module name in message[/-] [+]quote bad module name in ImportError-like messages[/+] on Sep 2, 2011
    21. brettcannon commented on Feb 9, 2012

      @brettcannon
      Member

      With bpo-1559549 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.

    22. merwok commented on Mar 2, 2012

      @merwok
      Member

      With bpo-1559549 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.

    23. brettcannon commented on Mar 7, 2012

      @brettcannon
      Member

      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.

    24. brettcannon commented on Feb 1, 2013

      @brettcannon
      Member

      Over time I have fixed this issue in various patches.

    25. merwok commented on Feb 1, 2013

      @merwok
      Member

      Great!

    26. transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    easystdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions