classification
Title: TypeError: __init__() takes at least 4 arguments (4 given)
Type: behavior Stage: resolved
Components: Documentation, Extension Modules Versions: Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: A. Skrobov, docs@python, nanjekyejoannah, paul.j3, r.david.murray
Priority: normal Keywords: 3.5regression

Created on 2015-10-02 11:25 by A. Skrobov, last changed 2019-08-18 00:42 by nanjekyejoannah.

Messages (16)
msg252106 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2015-10-02 11:25
Python 2.7.3 (default, Dec 18 2014, 19:10:20) 
[GCC 4.6.3] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from argparse import ArgumentParser
>>> parser = ArgumentParser()
>>> parser.add_argument("--foo", help="foo", action='store_const')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/argparse.py", line 1281, in add_argument
    action = action_class(**kwargs)
TypeError: __init__() takes at least 4 arguments (4 given)
>>> 



First, the exception message is entirely unhelpful (you wanted at least 4, you got 4, what's your problem?)

Second, adding "const=None" to the invocation makes it succeed; but,  according to https://docs.python.org/2/library/argparse.html#const this argument defaults to "None", so it shouldn't be necessary to specify it explicitly. Thus, either the documentation or the implementation is wrong.
msg252120 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-02 14:55
Yes, the python2 TypeErrors for mimatched arguments are sub-optimal.  This has been fixed in python3:

>>> parser.add_argument('--foo', help="foo", action='store_const')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/rdmurray/python/p36/Lib/argparse.py", line 1336, in add_argument
    action = action_class(**kwargs)
TypeError: __init__() missing 1 required positional argument: 'const'
msg252121 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-02 14:56
Oops, sorry, I missed the fact that you were pointing out a doc issue.
msg252123 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-02 15:02
Hmm.  After looking at the code, it may also be desirable to improve that error message at the argparse level, since there's a bit of not-immediately-obvious indirection going on there.
msg252124 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2015-10-02 15:05
Thank you for confirming that the mismatch between the documentation and the behaviour is preserved in Python 3!

Adding it to the list of affected versions.
msg252127 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-02 15:15
To clarify this for other readers: the issue here is that the arguments to add_argument are passed through the action routine, which in this case is store_const, and store_const is the one that requires 'const' be specified...which isn't made clear by either the docs or the error message.
msg252133 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-10-02 15:54
A related issue is

http://bugs.python.org/issue24754
argparse add_argument with action="store_true", type=bool should not crash

In this case the user specified a parameter that the 'store_true' subclass did not accept.

In both cases, 'parser.add_argument' does minimal error checking (or catching), so the user ends up seeing the '__init__' argument error.

This line in the 'store_const' documentation is clearly wrong.  There's no default.

    (Note that the const keyword argument defaults to the rather
    unhelpful None.)

Possible fixes to the bigger issue:

- major addition to the documentation, documenting allowable subclass parameters (but the subclasses aren't part of the API).

- major addition to add_argument that filters parameters before passing them to the subclasses.  But can that be done without replicating information that is implicit in the subclass __init__?  Can we use advanced inspection?  What about user defined Action classes?

- minor addition to add_argument that catches the __init__ parameter errors and adds a mollifing explanation.

- somehow make Action (and its subclasses) responsible for a nice error message.  But how do you override the normal behavior of Python regarding function parameters?
msg252135 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2015-10-02 15:58
I think your second to last choice (catch the error) is the only practical solution.  It is also probably the best, since we need to support user-written action routines.
msg252192 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-10-03 04:53
A fix that I am exploring would wrap the Action instantiation call in add_argument with a try/except block

That is replace:

    def add_argument(...)
        ....
        action = action_class(**kwargs)
        ...

with

        ....
        try:
            action = action_class(**kwargs)
        except TypeError:
            msg = str(_sys.exc_info()[1])
            msg = msg.replace('__init__','add_argument')
            msg = [msg]
            msg.append('Wrong argument(s) for Action subclass: %s'%action_class.__name__)
            # action_class is now a class, rather than the input string
            msg.append('kwargs: %s'%kwargs)
            sig = getattr(action_class, 'action_sig',None)
            if sig is not None:
                msg = sig(msg)
            raise ValueError(msg)
       ....

This collects information on the error, the action class and arguments, and passes them to a class method of the Action.  That customizes the message, and returns it for reraising.

In 'class Action' I define:

    @classmethod
    def action_sig(cls, msg=[]):
        # return the signature
        # subclasses may return custom version
        import inspect
        try:
            name = cls.__name__
            sig = inspect.signature(cls.__init__)
            sig = str(sig)
        except AttributeError:
            spec = inspect.getfullargspec(cls.__init__)
            sig = inspect.formatargspec(*spec)
        # remove self, option_strings, dest
        dstr='dest,'
        ind = sig.find(dstr)
        if ind>=0:
            sig = '(...'+sig[(ind+len(dstr)+1):]
        sig = 'class args: %s'%sig
        msg.append(sig)
        return '\n'.join(msg)

This adds inspect.signature information from the subclass __init__ method to the message from add_argument.  Subclasses (including the user defined ones) could customize this.

So a missing 'const' error would display as:

ValueError: add_argument() missing 1 required positional argument: 'const'
Wrong argument(s) for Action subclass: _StoreConstAction
kwargs: {'option_strings': ['--bar'], 'dest': 'bar'}
class args: (...const, default=None, required=False, help=None, metavar=None)

This may be overkill, but it gives an idea of the information that we can add to the original TypeError.

Done right this action_sig() could also serve as a usage function for an Action class.
msg252240 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2015-10-03 21:13
The unittest file test_argparse.py tests add_argument parameters in 

    class TestInvalidArgumentConstructors(TestCase)

It looks at a dozen different categories of errors, mostly checking that they return the correct TypeError or ValueError.  It does not check the content of error messages.  

If I change the code I described yesterday to return a TypeError again (but with an enhanced message), it passes this unittest.

Most of the test_argparse.py tests just check for error type.  There just a couple of late additions that check error message content:

    TestMessageContentError
    TestAddArgumentMetavar
msg349553 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python triager) Date: 2019-08-13 13:24
paul.j3, 

The fix looks promising. Do you want to open a PR with this fix?
msg349584 - (view) Author: paul j3 (paul.j3) * (Python triager) Date: 2019-08-13 16:42
I'm not set up to work with the current development distribution (via github).  All my proposed patches are diffs for earlier repos.

Paul
msg349878 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python triager) Date: 2019-08-16 17:26
> this argument defaults to "None"

Am leaning to the fact that this works as required. The documentation does not just say defaults to none. It says:

With the 'store_const' and 'append_const' actions, the const keyword argument must be given. For other actions, it defaults to None.

In the scenario, you have given, you are using 'store_const' so the const keyword argument must be given.

This should be closed If you agree with what I have just stated.
msg349899 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2019-08-17 13:42
Joannah, I see that under #25314, the docs were updated to match the implementation: https://github.com/python/cpython/commit/b4912b8ed367e540ee060fe912f841cc764fd293

On the other hand, the discussion here (from 2015) and on #25314 (from 2016) includes suggestions for improvements in the implementation as well.
msg349910 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python triager) Date: 2019-08-18 00:40
Since both the docs and implementation now match, I suggest we close this and open a new issue suggesting to change the implementation back to make const=None when action='store_const'.
msg349912 - (view) Author: Joannah Nanjekye (nanjekyejoannah) * (Python triager) Date: 2019-08-18 00:42
Opened issue37880 to track this change.
History
Date User Action Args
2019-08-18 00:42:03nanjekyejoannahsetmessages: + msg349912
2019-08-18 00:40:44nanjekyejoannahsetmessages: + msg349910
stage: needs patch -> resolved
2019-08-17 13:42:50A. Skrobovsetmessages: + msg349899
2019-08-16 17:26:02nanjekyejoannahsetmessages: + msg349878
2019-08-13 16:42:57paul.j3setmessages: + msg349584
2019-08-13 13:24:04nanjekyejoannahsetnosy: + nanjekyejoannah
messages: + msg349553
2015-10-03 21:13:17paul.j3setmessages: + msg252240
2015-10-03 04:53:45paul.j3setkeywords: + 3.5regression

messages: + msg252192
2015-10-02 15:58:55r.david.murraysetmessages: + msg252135
2015-10-02 15:55:00paul.j3setnosy: + paul.j3
messages: + msg252133
2015-10-02 15:15:15r.david.murraysetmessages: + msg252127
2015-10-02 15:05:34A. Skrobovsetmessages: + msg252124
versions: + Python 3.5
2015-10-02 15:02:11r.david.murraysetmessages: + msg252123
2015-10-02 14:56:25r.david.murraysetstatus: closed -> open
resolution: out of date ->
messages: + msg252121

stage: resolved -> needs patch
2015-10-02 14:55:15r.david.murraysetstatus: open -> closed

nosy: + r.david.murray
messages: + msg252120

resolution: out of date
stage: resolved
2015-10-02 11:25:46A. Skrobovcreate