Author arnau
Recipients arnau, bethard, eric.araujo
Date 2011-08-22.01:46:14
SpamBayes Score 1.05656e-09
Marked as misclassified No
Message-id <1313977575.82.0.633120941263.issue12776@psf.upfronthosting.co.za>
In-reply-to
Content
Thanks for the review. Sorry to send that here instead of the review page, but I get an error when replying: "Invalid XSRF token.".

> This looks good, especially if all existing tests still pass as you report, but
> I wonder about one thing: you have removed the part where the conversion
> function was applied to the default value, so I expected you to edit the other
> line were the conversion function was already called, but that’s not the case. 
> Am I misunderstanding something?

Yes, sorry, I should have perhaps explained it in further details... Here are some examples:

* Example test case 1:

parser = argparse.ArgumentParser()
parser.add_argument('--foo', type=type_foo_func, default='foo')
parser.parse_args('--foo bar'.split())

=> Before the patch, type function is called in parse_known_args() for the default given in add_argument(), and then in _parse_known_args() for '--foo bar' given in parse_args above, whereas type function should have been called only for the second one.

* Example test case 2:

parser = argparse.ArgumentParser()
parser.add_argument('--foo', type=type_foo_func)
parser.parse_args('--foo bar'.split())

=> This was already working well before my patch.

* Example test case 3:

parser = argparse.ArgumentParser()
parser.add_argument('--foo', type=type_foo_func, default='foo')
parser.parse_args('')

=> type_foo_func is called after parsing arguments (none in this case) in my patch.

Therefore, my patch just moves the function type call after parsing the arguments (given to parse_args()) instead of before, only and only if it was not previously given in parse_args().

> http://bugs.python.org/review/12776/diff/3181/9898#newcode1985
> Lib/argparse.py:1985: if hasattr(namespace, action.dest) and \
> It is recommended to use parens to group multi-line statements, backslashes are
> error-prone.

I have just updated the patch on the bug report. Thanks.
History
Date User Action Args
2011-08-22 01:46:15arnausetrecipients: + arnau, bethard, eric.araujo
2011-08-22 01:46:15arnausetmessageid: <1313977575.82.0.633120941263.issue12776@psf.upfronthosting.co.za>
2011-08-22 01:46:15arnaulinkissue12776 messages
2011-08-22 01:46:14arnaucreate