Change ImportError reference handling, naming

[briancurtin]

BPO	14600
Nosy	@pitrou, @merwok, @briancurtin
Files	ImportError_changes.diff issue14600.diff

^{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/briancurtin'
closed_at = <Date 2012-07-13.16:58:36.132>
created_at = <Date 2012-04-16.21:14:28.787>
labels = ['type-bug']
title = 'Change ImportError reference handling, naming'
updated_at = <Date 2012-07-13.16:58:36.131>
user = 'https://github.com/briancurtin'

bugs.python.org fields:

activity = <Date 2012-07-13.16:58:36.131>
actor = 'brian.curtin'
assignee = 'brian.curtin'
closed = True
closed_date = <Date 2012-07-13.16:58:36.132>
closer = 'brian.curtin'
components = []
creation = <Date 2012-04-16.21:14:28.787>
creator = 'brian.curtin'
dependencies = []
files = ['25245', '25248']
hgrepos = []
issue_num = 14600
keywords = ['patch']
message_count = 9.0
messages = ['158499', '158515', '158516', '158523', '158536', '158570', '158669', '165356', '165391']
nosy_count = 5.0
nosy_names = ['pitrou', 'eric.araujo', 'eli.bendersky', 'brian.curtin', 'python-dev']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue14600'
versions = ['Python 3.3']

[briancurtin]

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.

[pitrou]

Looks good on the principle. Implementation is a bit weird: you don't need to check name and path for NULL-ness?

[pitrou]

Oh, and is PyErr_SetExcWithArgsKwargs still useful?

[briancurtin]

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.

[pitrou]

You probably want to check args and kwargs for NULL-ness too. Otherwise, looks good.

[python-dev]

New changeset 7a32b9380ffd by Brian Curtin in branch 'default':
Fix bpo-14600. Correct reference handling and naming of ImportError convenience function
http://hg.python.org/cpython/rev/7a32b9380ffd

[merwok]

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.

[elibendersky]

Eric: your note appears to be fixed in the code.
Can this issue be closed?

[merwok]

Yep, bf23a6c215f6 fixed it, thanks for the ping. Brian or Antoine, can you close this or was there something else?