Repository navigation
ImportError needs attributes for module and file name #43980
Description
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/brettcannon'
closed_at = <Date 2012-04-13.00:27:13.367>
created_at = <Date 2006-09-15.19:55:04.000>
labels = ['interpreter-core', 'type-feature']
title = 'ImportError needs attributes for module and file name'
updated_at = <Date 2012-04-13.00:27:13.366>
user = 'https://github.com/nedbat'bugs.python.org fields:
activity = <Date 2012-04-13.00:27:13.366>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2012-04-13.00:27:13.367>
closer = 'brett.cannon'
components = ['Interpreter Core']
creation = <Date 2006-09-15.19:55:04.000>
creator = 'nedbat'
dependencies = []
files = ['21284', '23522', '24467', '24476']
hgrepos = []
issue_num = 1559549
keywords = ['patch']
message_count = 40.0
messages = ['54901', '54902', '125660', '125661', '125662', '130190', '130218', '130243', '130250', '130950', '131027', '131147', '131153', '131368', '140266', '140307', '142251', '146404', '149517', '149521', '149539', '149592', '149593', '149603', '149628', '152921', '152930', '152956', '152974', '152977', '152978', '153047', '153048', '153051', '153054', '153060', '153070', '158002', '158188', '158189']
nosy_count = 17.0
nosy_names = ['loewis', 'brett.cannon', 'ncoghlan', 'belopolsky', 'pitrou', 'vstinner', 'techtonik', 'giampaolo.rodola', 'nedbat', 'eric.araujo', 'r.david.murray', 'Trundle', 'brian.curtin', 'gruszczy', 'cool-RR', 'python-dev', 'eric.snow']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue1559549'
versions = ['Python 3.3']
Activity
nedbat commentedon Sep 15, 2006
Exceptions would be more useful if they had some
structured information attached to them. For example,
an ImportError could have the name of the module that
could not be imported. This would make it possible to
deal with exceptions in more powerful ways.
For more discussion:
http://www.nedbatchelder.com/blog/200609.html#e20060906T055924
loewis commentedon Sep 16, 2006
Logged In: YES
user_id=21627
Exceptions do have structured informations, for example
py> try:
... open("/tmp/xxx")
... except IOError, e:
... print e.__dict__
...
{'errno': 2, 'args': (2, 'No such file or directory'),
'strerror': 'No such file or directory', 'filename': '/tmp/xxx'}
It's just that ImportError doesn't, so I'm retitling this
request to restrict attention to ImportError.
If you have other proposals for specific information that
should be on specific exceptions, please submit a separate
issue.
Would you like to work on this specific problem? I think
ImportError should get a module attribute (always set), and
a filename attribute (set only if a file was selected, yet
failed to import; otherwise set to None). It probably will
require some refactoring of C code to simplify raising
ImportError in the importing code.
Explicit raises of ImportError should also be considered;
those in the standard library should be fixed to include
atleast the module; raising ImportError without giving a
module should set the module to None.
techtonik commentedon Jan 7, 2011
I think we should investigate deeper why this enhancement request didn't get into Python 3.
Another user story is: "from xxx import yyy", if yyy not found, the args message is ('cannot import name yyy',)
Python4?
label:api
bitdancer commentedon Jan 7, 2011
There's no need for any deeper investigation. The answer is "nobody wrote the patch". If someone writes a good patch, it will go in.
abalkin commentedon Jan 7, 2011
There is nothing to investigate here. This is a request for a marginal improvement and OP did not follow up even though a core developer responded on the next day after his post.
The only lesson to be learned from this is that Python improvements should be discussed on the tracker or appropriate python mailing lists and not on private blogs.
gruszczy commentedon Mar 6, 2011
This is a draft of a patch. I have only used this new ImportError api in once place, so it would work with following code:
I have literally no experience with Python core, so I would be very grateful for comments and advice, so I could make this patch meet all requirements.
Trundle commentedon Mar 7, 2011
There are some issues with the patch:
argsinImportError_init()is wrong: You can't create anImportErrorwith 3 arguments now ("TypeError: ImportError expected 2 arguments, got 3")ImportError_clear()doesn't clearself->msg.ImportError_str()doesn't increase the reference count forself->msgbefore returning it.PyUnicode_DecodeASCII()for the module name is wrong (IMHO), it should rather be something likePyUnicode_DecodeUTF8()as module names can contain non-ASCII characters in Python 3 and are AFAIK (at least right now) always encoded using utf-8.vstinner commentedon Mar 7, 2011
The module name is a UTF-8 encoded string yes. It should be documented in PyModuleDef structure. I already documented the encoding in PyModule_New().
gruszczy commentedon Mar 7, 2011
I am sorry again for those mistakes, it's all completely new to me. I have fixed those issues and created new patch. Using hg export, that now spans over two commits. Is it the way those patches should be provided, or should I gather all changes into a one commit?
Trundle commentedon Mar 15, 2011
No worries!
A single patch (where all changesets are flattened into one) is the
preferred way. For general advice, you can read the Developer's Guide
at http://docs.python.org/devguide/ (e.g.
http://docs.python.org/devguide/patch.html).
gruszczy commentedon Mar 15, 2011
I didn't know about this mq extension. I will do a proper patch with a test after friday.
brettcannon commentedon Mar 16, 2011
At the PyCon 2011 sprint we discussed this issue and Nick, myself, and some other people agreed that using a keyword-only argument for passing in the module name is probably a better solution. While it won't be backwards-compatible (BaseException does not accept keyword arguments), it does provide a very clean API with an unambiguous way of specifying the module. Going another route (e.g., a constructor method) has the same backwards-compatibility issue. But a reason to use a solution other than the magical handling of the second argument is that it prevents doing the wrong thing simply because someone passes two or more arguments to ImportError.
Another nicety of a new API for ImportError is that it can be made such that if the keyword-only argument ('module_name'?) is the only thing supplied (e.g., no positional arguments) then the message can be auto-generated, which would be a nice way to solve issue bpo-8754.
bitdancer commentedon Mar 16, 2011
+1 for providing a way to autogenerate the message.
19 remaining items
brettcannon commentedon Feb 9, 2012
Thanks, Brian! I'll do a review tonight on the drive home (and maybe even write the docs up).
bitdancer commentedon Feb 9, 2012
On the drive home...are you borrowing one of Google's self driving cars? :)
brettcannon commentedon Feb 10, 2012
The patch looks fine for its current semantics except for the fact that the macro doesn't work under clang; ##macro_var is supposed to be used only when concatenating parts of a string to create a complete identifier, not concatenating two identifiers next to each other (e.g. gooblede##gook is fine, but self->##gook isn't because '->' is its own identifier).
But one issue is that for my purposes I need 'name' to be the full name of the module, not just the tail of the full name. The reason is that pydoc needs a way to tell when a module import failed because the module isn't there vs. when a module had an actual error itself. Using the tail end of the name doesn't help me because it means I can't tell if an import of a module named 'bunk' containing the line 'import test.bunk' fails because the module 'bunk' doesn't exist or because the import line failed since in both cases name == 'bunk'. But if name == 'test.bunk' there is no ambiguity. Pydoc used to do this by inspecting the stack and seeing where the exception was thrown which is just nasty and brittle (i.e. doesn't work with importlib since the stack goes much farther until ImportError is thrown).
IOW I don't think we should be tossing out valuable data just because historically modules that didn't exist only returned the tail end of the full module name. Anyone object to switching to using the full name of the module triggering the exception? The message wouldn't change to use the full name (unfortunately) for backwards-compatibility reasons of course.
brettcannon commentedon Feb 10, 2012
And to answer David's joke, I carpool from Toronto to Waterloo four days a week so I have an hour each direction to work on stuff.
brettcannon commentedon Feb 10, 2012
Oh, and there are no forward declarations for the new functions added to errors.c
brettcannon commentedon Feb 10, 2012
Attached is Brian's patch with the macro fix and forward declarations.
brettcannon commentedon Feb 10, 2012
So I just tried to pass fullname in import.c and it didn't work (of course). Looks like even when I try to use fullname in find_module_path_list() and load_next() it is still leaving out the package name. Ugh. That means either refactoring import.c to get the full name in all places, not get the full name (and thus I'm still screwed), have importlib do something different than import.c in hopes that importlib replaces import.c before this goes public, or add a full_name attribute for importlib to fill in with the full details.
bitdancer commentedon Feb 10, 2012
However you do it, I'm very much in favor of having the full name available. I either wrote or fixed (I can't remember which) that stack walk in pydoc, and you are right, it is very very ugly. This would also be a big benefit for unittest, which currently *doesn't* do the stack walk and therefore generates incorrect error messages when sub-imports fail.
brettcannon commentedon Apr 11, 2012
This is now officially blocking my importlib bootstrap work (issue bpo-2377), so I have assigned this to myself to get done and I hope to get this committed within a week *without* updating Python/import.c and the requisite tests or pydoc (which I will make part of my merge instead). If Brian beats me to getting this checked in that's great, but otherwise I will get to it myself.
python-dev commentedon Apr 13, 2012
New changeset 6825fd9b00ed by Brett Cannon in branch 'default':
Issue bpo-1559549: Add 'name' and 'path' attributes to ImportError.
http://hg.python.org/cpython/rev/6825fd9b00ed
brettcannon commentedon Apr 13, 2012
Patch for the attributes is in (no use by import.c)! I'm going to do a separate commit for use by importlib and then fix pydoc in my bootstrap branch.