classification
Title: pydoc reports misleading failure if target module raises an ImportError
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: r.david.murray Nosy List: conf, exarkun, r.david.murray
Priority: normal Keywords: easy, patch

Created on 2009-02-12 21:16 by exarkun, last changed 2009-07-19 02:44 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
pydocs.diff conf, 2009-06-03 15:27
issue5230.patch r.david.murray, 2009-06-23 16:10
Messages (22)
msg81821 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-02-12 21:16
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.
msg88627 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-01 01:00
I've written a patch.
Hope you like it :)
msg88628 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-01 01:20
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.
msg88629 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-01 01:34
The same patch with whitespaces. Is it ok now?
msg88630 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-01 01:48
You are still missing a space between 'module' and 'named' ;)
msg88631 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-01 02:10
A new patch with an unit test and with whitespaces.
msg88638 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-01 11:45
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.
msg88639 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-01 12:29
Ok.
New patch that passes the unit tests and with a new unit test covering
inexistant nested modules.
msg88641 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-01 12:40
by the way, I was not acquainted with this unit test thing... sorry
msg88643 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-01 13:37
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.
msg88647 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-01 15:57
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.
msg88671 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-01 20:19
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.
msg88704 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-02 01:18
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.
msg88739 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-02 11:54
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)
msg88740 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-02 13:10
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).
msg88814 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-03 15:27
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.
msg89014 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-06 19:46
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.
msg89618 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-23 01:01
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.
msg89628 - (view) Author: Lucas Prado Melo (conf) Date: 2009-06-23 15:27
I think this patch is ok.
msg89630 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-23 16:10
Here is an updated patch that cleans up the unit test (I wasn't
correctly restoring sys.path).
msg89703 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-06-25 12:27
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.
msg90700 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-07-19 02:44
Benjamin merged this to py3k/3.1 as part of r73623/r73625.
History
Date User Action Args
2009-07-19 02:44:42r.david.murraysetstatus: open -> closed

messages: + msg90700
2009-06-25 12:27:17r.david.murraysetresolution: fixed
stage: patch review -> resolved
messages: + msg89703
versions: - Python 2.6, Python 2.7
2009-06-23 16:10:58r.david.murraysetfiles: + issue5230.patch

messages: + msg89630
2009-06-23 16:07:30r.david.murraysetfiles: - issue5230.patch
2009-06-23 15:27:21confsetmessages: + msg89628
2009-06-23 01:02:03r.david.murraysetfiles: - issue5230.patch
2009-06-23 01:01:54r.david.murraysetfiles: + issue5230.patch
assignee: r.david.murray
messages: + msg89618

versions: + Python 3.2, - Python 3.0
2009-06-06 19:46:03r.david.murraysetfiles: + issue5230.patch

messages: + msg89014
2009-06-03 15:30:46confsetfiles: - pydocs.diff
2009-06-03 15:27:09confsetfiles: + pydocs.diff

messages: + msg88814
2009-06-02 13:10:47r.david.murraysetmessages: + msg88740
2009-06-02 11:54:34confsetmessages: + msg88739
2009-06-02 01:18:20confsetmessages: + msg88704
2009-06-01 20:19:55r.david.murraysetmessages: + msg88671
stage: test needed -> patch review
2009-06-01 15:57:38confsetmessages: + msg88647
2009-06-01 13:37:22r.david.murraysetmessages: + msg88643
2009-06-01 12:40:45confsetmessages: + msg88641
2009-06-01 12:29:34confsetfiles: - pydocs.diff
2009-06-01 12:29:25confsetfiles: + pydocs.diff

messages: + msg88639
2009-06-01 11:45:19r.david.murraysetmessages: + msg88638
2009-06-01 02:10:56confsetfiles: - pydocs.diff
2009-06-01 02:10:38confsetfiles: + pydocs.diff

messages: + msg88631
2009-06-01 01:49:00r.david.murraysetmessages: + msg88630
2009-06-01 01:35:02confsetfiles: - pydocs.diff
2009-06-01 01:34:49confsetfiles: + pydocs.diff

messages: + msg88629
2009-06-01 01:20:42r.david.murraysetversions: + Python 3.1, Python 2.7
nosy: + r.david.murray

messages: + msg88628

components: + Library (Lib)
stage: needs patch -> test needed
2009-06-01 01:00:28confsetfiles: + pydocs.diff

nosy: + conf
messages: + msg88627

keywords: + patch
2009-04-22 14:40:44ajaksu2setpriority: normal
keywords: + easy
versions: + Python 2.6, Python 3.0
2009-02-17 02:07:30benjamin.petersonsetstage: needs patch
2009-02-12 21:16:27exarkuncreate