Skip to content
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

A more helpful ImportError message #73732

Closed
warsaw opened this issue Feb 13, 2017 · 8 comments
Closed

A more helpful ImportError message #73732

warsaw opened this issue Feb 13, 2017 · 8 comments
Labels
3.7 interpreter-core type-feature

Comments

@warsaw
Copy link
Member

@warsaw warsaw commented Feb 13, 2017

BPO 29546
Nosy @warsaw, @brettcannon, @ilevkivskyi, @Carreau
PRs
  • #91
  • #91
  • #91
  • #103
  • #552
  • 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 = None
    closed_at = <Date 2017-02-22.15:08:42.389>
    created_at = <Date 2017-02-13.16:30:11.090>
    labels = ['interpreter-core', 'type-feature', '3.7']
    title = 'A more helpful ImportError message'
    updated_at = <Date 2017-03-31.16:36:25.272>
    user = 'https://github.com/warsaw'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:25.272>
    actor = 'dstufft'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-02-22.15:08:42.389>
    closer = 'barry'
    components = ['Interpreter Core']
    creation = <Date 2017-02-13.16:30:11.090>
    creator = 'barry'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 29546
    keywords = []
    message_count = 8.0
    messages = ['287708', '287719', '287741', '287802', '287803', '287804', '287805', '290430']
    nosy_count = 4.0
    nosy_names = ['barry', 'brett.cannon', 'levkivskyi', 'mbussonn']
    pr_nums = ['91', '91', '91', '103', '552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue29546'
    versions = ['Python 3.7']

    @warsaw
    Copy link
    Member Author

    @warsaw warsaw commented Feb 13, 2017

    I haven't really thought about this deeply but I've observed that there are lots of cases where a user will report getting an ImportError trying to import a name from a module, where it turns out that the problem is that the module is coming from an unexpected location. Examples include pip installed packages overriding system packages, or unexpected vendorized wheels.

    The standard first response is typically, "can you please run this to tell us where the foo library is coming from?" E.g.

    ^^ this is indicative of an old version of urllib3. Can you please double 
    check whether you have an old urllib3 installed e.g. in /usr/local or ~/.
    
    Testing:
    
    $ python3 -c 'import urllib3; print(urllib3.__file__)'
    
    should return:
    
    	/usr/lib/python3/dist-packages/urllib3/__init__.py
    
    and might tell you where else a locally installed urllib3 is to be found if it 
    doesn't return that value.
    

    It would be kind of useful if the original ImportError showed you where the module came from (i.e. its __file__), if such information can be discerned. E.g.:

    >>> import requests; requests.__version__.split('.')  
    Traceback (most recent call last):
       File "<stdin>", line 1, in <module>
       File "/usr/lib/python2.7/dist-packages/requests/__init__.py", line 
    61, in <module>
         from .packages.urllib3.exceptions import DependencyWarning
    ImportError: cannot import name DependencyWarning from requests.packages.urllib3.exceptions (/usr/local/lib/python3.5/site-packages/requests/packages/urllib3.py)
    

    If you saw that in a bug report, you'd almost immediately know that the user has some local override that's potentially causing the problem.

    @warsaw warsaw added 3.7 interpreter-core labels Feb 13, 2017
    @brettcannon
    Copy link
    Member

    @brettcannon brettcannon commented Feb 13, 2017

    So 'path' already exists on ImportError so tweaking the message when the path is known wouldn't be difficult (https://docs.python.org/3/library/exceptions.html?highlight=importerror#ImportError).

    @brettcannon brettcannon added the type-feature label Feb 13, 2017
    @Carreau
    Copy link
    Mannequin

    @Carreau Carreau mannequin commented Feb 14, 2017

    Unsure if GitHub PR autolinks here.

    I'm attempting part of that at #91

    AFAICT with my little knowledge of CPython internal, name and path where unset for from ... import.

    I'll be happy to change the formatting as well but I suspect this will lead to lot of bikeshedding.

    @brettcannon
    Copy link
    Member

    @brettcannon brettcannon commented Feb 15, 2017

    New changeset bc4bed4 by Brett Cannon in branch 'master':
    bpo-29546: Set 'path' on ImportError for from ... import ... (GH-91)
    bc4bed4

    @brettcannon
    Copy link
    Member

    @brettcannon brettcannon commented Feb 15, 2017

    Thanks to Matthias' PR the information is all there in the exception, but the message has not been changed. One idea for this -- depending on how much C code you want to write -- is to provide a default message for __str__() that changes depending on whether 'path' and/or 'name' are set. Then you can just set the attributes in the __init__() and have __str__() take care of providing a common message format. Another option is to do all of that in the __init__() so that BaseException.args continues to have the full error message (but that is added overhead if the __str__() is never taken of the exception). I also have no clue how much C code this would take :) (This is all why I have toyed with the idea of re-implementing the exceptions in Python for easier customization.)

    @Carreau
    Copy link
    Mannequin

    @Carreau Carreau mannequin commented Feb 15, 2017

    I'm unsure I understand changing only the default __str__() method. You will anyway have to format the message differently depending on whether you raise from a from-import or a from-import-* or any other locations.

    AFAIU you "just" need the following

    •    PyErr_SetImportError(PyUnicode_FromFormat("cannot import name %R", name), pkgname, pkgpath);
      

    + PyErr_SetImportError(
    + PyUnicode_FromFormat("cannot import name %R from %R (%S)",
    + name, pkgname, pkgpath),
    + pkgname, pkgpath);

    To use Barry format (though keeping quotes around identifiers to match current behavior).
    (And similar if path is null).

    I'm unsure if you meant to provide a set of "format-template" to ImportError that are guarantied to be called with format(name=..., path=...) but I doubt it.

    @Carreau
    Copy link
    Mannequin

    @Carreau Carreau mannequin commented Feb 15, 2017

    See #103 that implements Barry's proposed format.

    @warsaw warsaw closed this as completed Feb 22, 2017
    @warsaw
    Copy link
    Member Author

    @warsaw warsaw commented Mar 24, 2017

    New changeset 1bc1564 by Barry Warsaw (Matthias Bussonnier) in branch 'master':
    bpo-29546: Improve from-import error message with location (#103)
    1bc1564

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 interpreter-core type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants