classification
Title: Change ImportError reference handling, naming
Type: behavior Stage: resolved
Components: Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: brian.curtin Nosy List: brian.curtin, eli.bendersky, eric.araujo, pitrou, python-dev
Priority: high Keywords: patch

Created on 2012-04-16 21:14 by brian.curtin, last changed 2012-07-13 16:58 by brian.curtin. This issue is now closed.

Files
File name Uploaded Description Edit
ImportError_changes.diff brian.curtin, 2012-04-16 21:14 review
issue14600.diff brian.curtin, 2012-04-17 02:43
Messages (9)
msg158499 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-04-16 21:14
Antoine mentioned in email that the reference handling should be changed, so here's a shot at it. I also condensed and renamed the convenience functions - I was paying too much attention to the surrounding conventions and made this harder than it had to be.
msg158515 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-17 00:30
Looks good on the principle. Implementation is a bit weird: you don't need to check name and path for NULL-ness?
msg158516 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-17 00:30
Oh, and is PyErr_SetExcWithArgsKwargs still useful?
msg158523 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2012-04-17 02:43
How about this patch? Adds NULL checking and merges PyErr_SetExcWithArgsKwargs inside PyErr_SetImportError since it's not needed by itself. Docs are also updated in line with these changes.
msg158536 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-17 10:21
You probably want to check args and kwargs for NULL-ness too. Otherwise, looks good.
msg158570 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-04-17 21:57
New changeset 7a32b9380ffd by Brian Curtin in branch 'default':
Fix #14600. Correct reference handling and naming of ImportError convenience function
http://hg.python.org/cpython/rev/7a32b9380ffd
msg158669 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-04-18 22:05
As was pointed on python-dev for the first commit:

    args = PyTuple_New(1);
    if (args == NULL)
        return NULL;

    kwargs = PyDict_New();
    if (args == NULL)
        return NULL;

It looks like the second block has a copy-paste typo and should check kwargs, not args.
msg165356 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2012-07-13 05:29
Eric: your note appears to be fixed in the code.
Can this issue be closed?
msg165391 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-07-13 16:15
Yep, bf23a6c215f6 fixed it, thanks for the ping.  Brian or Antoine, can you close this or was there something else?
History
Date User Action Args
2012-07-13 16:58:36brian.curtinsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2012-07-13 16:15:58eric.araujosetmessages: + msg165391
2012-07-13 05:29:09eli.benderskysetnosy: + eli.bendersky
messages: + msg165356
2012-04-18 22:05:28eric.araujosetnosy: + eric.araujo
messages: + msg158669
2012-04-17 21:57:31python-devsetnosy: + python-dev
messages: + msg158570
2012-04-17 10:28:54pitroulinkissue14593 superseder
2012-04-17 10:21:54pitrousetmessages: + msg158536
2012-04-17 02:43:07brian.curtinsetfiles: + issue14600.diff

messages: + msg158523
2012-04-17 00:30:48pitrousetmessages: + msg158516
2012-04-17 00:30:18pitrousetmessages: + msg158515
2012-04-16 21:14:28brian.curtincreate