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

pydoc reports misleading failure if target module raises an ImportError #49480

Closed
exarkun mannequin opened this issue Feb 12, 2009 · 22 comments
Closed

pydoc reports misleading failure if target module raises an ImportError #49480

exarkun mannequin opened this issue Feb 12, 2009 · 22 comments
Assignees
Labels
easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@exarkun
Copy link
Mannequin

exarkun mannequin commented Feb 12, 2009

BPO 5230
Nosy @bitdancer
Files
  • pydocs.diff
  • issue5230.patch
  • 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/bitdancer'
    closed_at = <Date 2009-07-19.02:44:42.859>
    created_at = <Date 2009-02-12.21:16:27.881>
    labels = ['easy', 'type-bug', 'library']
    title = 'pydoc reports misleading failure if target module raises an ImportError'
    updated_at = <Date 2009-07-19.02:44:42.858>
    user = 'https://bugs.python.org/exarkun'

    bugs.python.org fields:

    activity = <Date 2009-07-19.02:44:42.858>
    actor = 'r.david.murray'
    assignee = 'r.david.murray'
    closed = True
    closed_date = <Date 2009-07-19.02:44:42.859>
    closer = 'r.david.murray'
    components = ['Library (Lib)']
    creation = <Date 2009-02-12.21:16:27.881>
    creator = 'exarkun'
    dependencies = []
    files = ['14172', '14343']
    hgrepos = []
    issue_num = 5230
    keywords = ['patch', 'easy']
    message_count = 22.0
    messages = ['81821', '88627', '88628', '88629', '88630', '88631', '88638', '88639', '88641', '88643', '88647', '88671', '88704', '88739', '88740', '88814', '89014', '89618', '89628', '89630', '89703', '90700']
    nosy_count = 3.0
    nosy_names = ['exarkun', 'r.david.murray', 'conf']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue5230'
    versions = ['Python 3.1', 'Python 3.2']

    @exarkun
    Copy link
    Mannequin Author

    exarkun mannequin commented Feb 12, 2009

    If pydoc is used to try to look up the documentation for a module which
    does not exist, this is reported reasonably:

    exarkun@charm:~$ pydoc foobarbaz
    no Python documentation found for 'foobarbaz'
    
    exarkun@charm:~$
    

    However, if it is used to look up the documentation for a module which
    does exist but which itself imports a module which does not exist, then
    the same report is made:

    exarkun@charm:~$ cat > foobarbaz.py
    import barbazquux
    exarkun@charm:~$ pydoc foobarbaz
    no Python documentation found for 'foobarbaz'
    
    exarkun@charm:~$
    

    This is somewhat misleading, since it suggests that there is no such
    module "foobarbaz", when the problem is solved by installing the module
    "barbazquux":

    exarkun@charm:~$ touch barbazquux.py
    exarkun@charm:~$ pydoc foobarbaz
    Help on module foobarbaz:
    
    ...
    exarkun@charm:~$ 
    

    I'm sure the cause of this is the incautious handling of ImportError by
    pydoc, resulting in the mistaken identification of the problem.

    @exarkun exarkun mannequin added the type-bug An unexpected behavior, bug, or error label Feb 12, 2009
    @devdanzin devdanzin mannequin added the easy label Apr 22, 2009
    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 1, 2009

    I've written a patch.
    Hope you like it :)

    @bitdancer
    Copy link
    Member

    Thanks :)

    PEP-8 recommends spaces after commas in a list.

    Also, we should have a unit tests before we commit the fix. If you feel
    like writing them that would be most welcome.

    @bitdancer bitdancer added the stdlib Python modules in the Lib dir label Jun 1, 2009
    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 1, 2009

    The same patch with whitespaces. Is it ok now?

    @bitdancer
    Copy link
    Member

    You are still missing a space between 'module' and 'named' ;)

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 1, 2009

    A new patch with an unit test and with whitespaces.

    @bitdancer
    Copy link
    Member

    Thanks. The unit tests don't pass, though, at least not when I apply
    the patch to trunk. I get three failures. So the patch isn't correct
    as it stands. 'path' is the full module path (test.i_am_not_here in the
    case of one of the test failures), but what appears in the error message
    is just the name of the module that wasn't found.

    Also consider the case where the import is 'import some.nested.module',
    and the module that isn't found is 'nested'. In fact it may be worth
    writing an additional test for that case.

    Also I'd recommend renaming badimport_module.py pydoc_badimport.py so as
    to be parallel to the other pydoc auxiliary test files.

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 1, 2009

    Ok.
    New patch that passes the unit tests and with a new unit test covering
    inexistant nested modules.

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 1, 2009

    by the way, I was not acquainted with this unit test thing... sorry

    @bitdancer
    Copy link
    Member

    No need to apologize, and thank you for taking the time to learn this
    stuff. (Six months ago I didn't know how the python unit test suite
    worked either...and I keep learning new things.)

    For me your new test fails...and it isn't quite the one I had in mind.
    The test fails because with your patch pydoc correctly reports that
    there is no documentation for the non-existent temrinal module, while
    your test is expecting it to report a missing module.

    (This makes me wonder...is the existing behavior of pydoc optimal? With
    this patch in place would it be better to report that there is no such
    module rather than that there is no documentation found? But let's
    ignore that issue for the moment since this patch is required even if we
    were to change that message.)

    The test I had in mind would be a file pydoc_badimport2.py containing:

        import test.i_dont_exist.neither_do_i

    In that case, your patch will fail, because the error message will
    report that "i_dont_exist" can't be found.

    It's funny how these seemingly simple things turn out to be not quite so
    simple. Perhaps we could extract the last token (the module name) from
    the error message, and only do the "no doc" message if it is equal to
    the last element of the path name.

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 1, 2009

    I didn't understand what you mean: what should be shown when pydoc tries
    to generate documentation for a module with the bad import 'import
    test.i_dont_exist.neither_do_i'?
    I am not a native english speaker, so please excuse me if you don't
    understand something I've written or the other way around.

    @bitdancer
    Copy link
    Member

    It should generate:

    problem in pydoc_badimport2 - <type 'exceptions.ImportError'>: No
    module named i_dont_exist"

    If you'd like I can do the final fixup and apply the patch. I'm happy
    to let you get it working if you want, though, and I appreciate the work
    you've done so far either way.

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 2, 2009

    Thanks :)
    It seems that the error message carried by the ImportError object comes
    from Python/import.c:1504.
    What should we do:
    a) Edit Python/import.c
    b) Change the ImportError object
    c) Anything else.

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 2, 2009

    Take a look at the output:
    $ python pydoc.py test.pydoc_badimport2
    problem in test.pydoc_badimport2 - <type 'exceptions.ImportError'>: No
    module named i_dont_exist.neither_do_i

    This is different from what you expected. How do we change this output?
    (I was talking about this issue in the last message)

    @bitdancer
    Copy link
    Member

    I'm sorry, I mistyped. That is the error message I was expecting (it's
    derived from the one 'import' gives in that case). If the test were
    like this:

    elif exc is ImportError and str(value).endswith(path.split('.')[-1]):
    

    then I think the tests would pass (but I haven't checked yet).

    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 3, 2009

    I had lots of stuff to do lately, sorry it took me so long to answer.
    Here is the patch as we intended, but there is a bug yet.
    What if the non-existent imported module has the same name of the module
    itself?

    $ cat pydoc_badimport3.py
    import this_doesnt_exist.pydoc_badimport3
    $ pydoc pydoc_badimport3

    I tested this possibility, and I found out that there is a bug in this
    situation yet: pydoc still tells the user that the module couldn't be found.

    @bitdancer
    Copy link
    Member

    I've uploaded a new version of the patch and test suite. I wanted a few
    more test cases but didn't want to litter the test directory with little
    test files, so I borrowed some techniques from test_import and created
    the modules to import on the fly.

    I haven't come up with a fix for your last test case (yet?). I've
    commented the test to indicate this. I think to fix it we may need to
    look into the traceback itself.

    @bitdancer
    Copy link
    Member

    OK, I finally had time to come back to this, and figured out what I
    think is a final fix. It passes all the tests we've come up with, at
    least. Let me know if you see any problems with it, and if not I'll
    apply it.

    @bitdancer bitdancer self-assigned this Jun 23, 2009
    @conf
    Copy link
    Mannequin

    conf mannequin commented Jun 23, 2009

    I think this patch is ok.

    @bitdancer
    Copy link
    Member

    Here is an updated patch that cleans up the unit test (I wasn't
    correctly restoring sys.path).

    @bitdancer
    Copy link
    Member

    Applied to 2.7 in r73529 and 2.6 in r73530. Leaving ticket open until I
    can apply it to 3.1 and 3.2.

    Thanks for your help, Lucas.

    @bitdancer
    Copy link
    Member

    Benjamin merged this to py3k/3.1 as part of r73623/r73625.

    @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
    easy stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant