classification
Title: float arguments in scientific notation not supported by argparse
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4, Python 3.5
process
Status: closed Resolution: duplicate
Dependencies: Superseder: argparse does not accept options taking arguments beginning with dash (regression from optparse)
View: 9334
Assigned To: Nosy List: abacabadabacaba, bethard, jnespolo, paul.j3, r.david.murray, terry.reedy
Priority: normal Keywords: patch

Created on 2014-10-19 18:10 by jnespolo, last changed 2015-03-28 11:51 by abacabadabacaba. This issue is now closed.

Files
File name Uploaded Description Edit
argparse.patch jnespolo, 2014-10-19 18:10 patch to be applied on argparse.py
argparse.patch jnespolo, 2014-10-24 21:09 patch v2
test_argparse.py jnespolo, 2014-10-24 21:10 test program
Messages (9)
msg229689 - (view) Author: Jacopo Nespolo (jnespolo) * Date: 2014-10-19 18:10
Argparse fails to recognise negative numbers in scientific notation as valid numbers.

Example:
Suppose the program test.py has this option.
    par.add_argument('-a', type=float)

then './test.py -a -1e5' will fail, as well as -1.0e-4, -.5E+4 and variations thereof.
Furthermore, at the current state, it seems that argparse does not recognize -1. as a valid float either.

I tried to hack argparse.py myself, and I believe the patch attached should fix this issue. The base version of argparse.py is the one from Python 3.4.2 as found in Debian Sid.

The modified regular expression seemed to behave correctly in all test cases I could come up with.
msg229958 - (view) Author: Jacopo Nespolo (jnespolo) * Date: 2014-10-24 21:09
a better patch
msg229959 - (view) Author: Jacopo Nespolo (jnespolo) * Date: 2014-10-24 21:10
test program that checks handling of a bunch of numbers.
msg229960 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2014-10-24 21:22
Thanks for the updated patch and test program.  Note, to be part of the Python test suite, the test should be in the form of a unittest test and fit into the existing argparse tests here: Lib/test/test_argparse.py.
msg229985 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-10-25 05:21
This issue has already been raise in

http://bugs.python.org/issue9334
argparse does not accept options taking arguments beginning with dash (regression from optparse)

There I proposed leaving '_negative_number_matcher' unchanged, but only use it to set '_has_negative_number_optionals'.  Testing whether argument strings are numbers or not is better done with 'complex()'.


        if not self._has_negative_number_optionals:
            try:
                complex(arg_string)
                return None
            except ValueError:
                pass

That was my first patch, and I haven't looked at it much since then, other than testing that it is compatible with a number of other proposed patches.
msg229990 - (view) Author: Jacopo Nespolo (jnespolo) * Date: 2014-10-25 09:43
Ned Deily: I don't quite know how to use unittest, but I'll try to look into it.

paul j3:
> There I proposed leaving '_negative_number_matcher' unchanged, 
> but only use it to set '_has_negative_number_optionals'.

I don't know argparse internals but, if I understand your argument, I think you still need a '_negative_number_matcher' capable of correctly match any negative number in any form, including scientific notation, complex j, etc..
If this is the case, then, the matcher could be simplified to something like

    '^-\d+|^-.\d+'

(notice $ removed from current regex) which would only signal that "something that starts like a negative number" is present.
Then one could use complex() or float() or whatever, to check that that's actually the case.

I would still expect an exception to be raised if, say, I specify type=float and then a complex value is passed as an argument.
msg230018 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2014-10-25 21:42
Patch tests are added to an existing 'Lib/test/test_argparse.py' file.  I use existing test cases as a pattern any new tests.

---------------------

Your test file runs fine with the patch I proposed for Issue 9334.

---------------------

The argparse code uses '_negative_number_matcher' for 2 purposes

1) to test whether an option_string looks like a negative number, and set the 'self._has_negative_number_optionals' attribute.

    parser.add_argument('-2')
    parser.add_argument('-1.234')

2) to test whether an argument string (an element of sys.argv) looks like one of those option_strings, or is an argument (positional or argument to your '-a').

The 'type' in for your '-a' argument is separate issue.  That is used convert a string to a number, float, complex or what ever, and raise an error it if can't do so.

In your case './test.py -a -1e5' fails because '-1e5' fails the 2nd test, and thus is not recognized as an argument to '-a'. Understanding the details of this requires digging into the logic of the _parse_optional() method.

'./test.py -a-1e5' or './test.py -a=-1e5' work because the number is correctly recognized as an argument.

For issue 9334 I looked at generalizing '_negative_number_matcher' as you did.  But unless you want to use something like:

    parser.add_argument('-1.2e-34')

and set the 'self._has_negative_number_optionals' to '[True]', the matcher doesn't need to be more general.

It's only the test in '_parse_optional()' that needs to be generalized to handle scientific and complex notation.  And for that I figured that wrapping 'complex()' in a 'try' block was just as general and reliable as a complicated 're' pattern.  At least that was my claim in issue 9334, and I haven't gotten feedback on that.

I'd suggest reading the 9334 discussion, and testing that patch.  That patch includes some tests for scientific and complex numbers.

That issue and patch also adds a 'args_default_to_positional' parameter. I wonder if the two changes should be put in separate patches.
msg230172 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-10-28 19:42
If this issue is a duplicate of #9334, as it appears to be, this should be closed and a note added to #9334 that there is a patch here also.
msg239436 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-03-27 20:52
closed with reference to #9334
History
Date User Action Args
2015-03-28 11:51:22abacabadabacabasetnosy: + abacabadabacaba
2015-03-27 21:10:13terry.reedysetsuperseder: argparse does not accept options taking arguments beginning with dash (regression from optparse)
resolution: duplicate
stage: patch review -> resolved
2015-03-27 20:52:03paul.j3setstatus: open -> closed

messages: + msg239436
2014-12-16 06:42:48ned.deilysetnosy: - ned.deily
2014-10-28 19:42:22terry.reedysetnosy: + terry.reedy
messages: + msg230172
2014-10-25 21:42:37paul.j3setmessages: + msg230018
2014-10-25 09:43:39jnespolosetmessages: + msg229990
2014-10-25 05:21:19paul.j3setnosy: + paul.j3, r.david.murray
messages: + msg229985
2014-10-24 21:22:55ned.deilysetstage: patch review
versions: + Python 3.5
2014-10-24 21:22:11ned.deilysetnosy: + ned.deily
messages: + msg229960
2014-10-24 21:10:32jnespolosetfiles: + test_argparse.py

messages: + msg229959
2014-10-24 21:09:29jnespolosetfiles: + argparse.patch

messages: + msg229958
2014-10-22 19:53:04jnespolosetnosy: + bethard
2014-10-19 18:10:15jnespolocreate