Document the Action API in argparse

[jaraco]

BPO	13540
Nosy	@terryjreedy, @jaraco, @merwok
Files	956c6d33a57d.diff b8c09b766e20.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 = None
closed_at = <Date 2014-08-03.19:34:22.913>
created_at = <Date 2011-12-06.14:56:34.958>
labels = ['type-bug', 'docs']
title = 'Document the Action API in argparse'
updated_at = <Date 2014-08-24.05:14:28.404>
user = 'https://github.com/jaraco'

bugs.python.org fields:

activity = <Date 2014-08-24.05:14:28.404>
actor = 'paul.j3'
assignee = 'docs@python'
closed = True
closed_date = <Date 2014-08-03.19:34:22.913>
closer = 'jaraco'
components = ['Documentation']
creation = <Date 2011-12-06.14:56:34.958>
creator = 'jaraco'
dependencies = []
files = ['23949', '36003']
hgrepos = ['95']
issue_num = 13540
keywords = ['patch']
message_count = 23.0
messages = ['148921', '149363', '149391', '149422', '149423', '149450', '149524', '223241', '223273', '223514', '223515', '223523', '224650', '224651', '224652', '224653', '225631', '225756', '225757', '225780', '225788', '225789', '225794']
nosy_count = 9.0
nosy_names = ['terry.reedy', 'jaraco', 'bethard', 'eric.araujo', 'docs@python', 'BreamoreBoy', 'tshepang', 'python-dev', 'paul.j3']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue13540'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[jaraco]

In http://docs.python.org/dev/library/argparse.html#action, when describing an arbitrary action, the documentation states, "You can also specify an arbitrary action by passing an object that implements the Action API." This statement does not link to any documentation on the Action API and does not describe the API except to give an example.

Furthermore, the example contradicts the description. The description says "pass an object" but the example passes a class.

The documentation should clarify the text relating to the example and should document the expected interface for a custom action.

[terryjreedy]

The doc specified the Action API as the interface inherited from argparse.Action plus the addition of a custom __call__ method with 4 params (5 with self) as described. That seems mostly adequate to me, unless there is something missing in the parameter descriptions.

I agree that it should be stated if one should pass the class rather than an instance thereof. (But classes are objects, so that is not a 'contradiction'.) If the example is in error, it should be fixed.

[jaraco]

You're right. The documentation isn't incorrect, if you're splitting hairs. But it's not super friendly either.

Questions that the documentation should answer:

1. Does the action always need to be a subclass of an Action, or is that recommended? If it's recommended, replace "easiest" with "recommended".
2. If it does not always need to be a subclass of Action, what is the interface expected of a class for a custom action?
3. If one does use a subclass of Action, are there any hooks that are called at any point? How does one customize an action other than to override __call__? What attributes are available on the object (the example shows .dest only)?
4. What is the action required to do in __call__, if anything? Is there any way to invoke the default behavior (if for example a special case wasn't detected)?

All of these questions can be answered by going to the source, but I've twice now gone to the documentation to reference how to provide a custom action and found the documentation insufficient.

Now that I review the source again, I think I know the answers to those questions.

1. It does not always need to be a subclass of Action, but it is recommended.
2. The action API expects a callable with the following signature: Action(option_strings, dest, nargs=None, const=None, default=None, type=None, choices=None, required=False, help=None, metavar=None) which when called returns another callable accepting the four parameters.
3. Argparse does nothing with the custom action except to call it first with the init parameters, then calls the result with four parameters. If subclassing Action, the default __init__ simply stores each of those parameters as attributes on the object. Most subclasses of Action will override the __init__ to validate the options and raise an exception (typically a ValueError) when the parameters aren't valid.
4. The action is not required to do anything. Most actions will perform some logic and then invoke setattr(namespace, self.dest, result) where result is the result of some logic.

I can see why no one wanted to write this documentation. It's difficult to describe simply.

Here's what I propose:

First, remove the phrase "Action API" which suggests that there's an API outside of subclassing the Action that's recommended.

Second, change "easiest" to "recommended" to using the class.

Third, explain what attributes are available on an object initialized from a subclass of Action. Provide a link to code or other section that describes advanced usage (overriding __init__).

Fourth, explain what is expected when calling an Action instance (i.e. setattr).

Fifth, consider extending the Action class so that __call__ calls an actual named method so that those writing custom actions aren't writing classes with only a __call__ method. Perhaps "invoke". I feel less strongly about this point, but it seems strange that the recommended usage is to write a class with only a __call__ method.

[jaraco]

I've attached a repository and patch with the recommended changes. I created an additional section that includes the documentation for the Action class (specifically the __init__ and __call__ signatures).

I believe this addresses the issues I raised. Any objections to this change? I haven't tested the rendering yet, but I'll do that before pushing to the master if it's adequate.

[terryjreedy]

My guess from the way the docs are written now is that subclassing from Action and over-riding just the __call__ method is the intended way, not just the recommended. If so, Action is a quasi-private class, only exposed so it could be subclassed with one method given. And if so, one could question whether anything more need be documented.

However, in your patch, you write

"+Many actions also override the __init__ method, validating the parameters to the argument definition and raising a ValueError or other Exception on failure."

What 'many actions' are you referring to? Ones you have written, by referring to the code? Ones you think might be written if the doc were added?

In any case, I would prefer input from Stephen before we expose and thereby freeze the __init__ signature.

[jaraco]

Most of the Action subclasses in argparse override __init__ and they raise ValueErrors when the parameters aren't as expected for that argument. This was my reason for adding that comment. If the basic Actions require this level of validation, wouldn't a custom action also want to supply this type of behavior? I'm sure I've overridden Action.__init__ in some of the subclasses I've created, but I don't remember specifically why.

I agree freezing the __init__ signature is undesirable. It would be preferable if the API called a hook for validating the argument parameters against the action. My intention, however, was to document the existing behavior, not influence changes in the behavior.

Perhaps the recommended API really isn't to subclass Action, but to supply a callable that takes the __init__ args which validates the parameters and returns a callable which takes the __call__ args which should set attributes on the namespace. Perhaps the Action class is only one such implementation of the API. Indeed, that was my understanding of the API as it is currently documented before Terry suggested otherwise.

What do you think about instead of documenting the Action class, we formalize the API, similar to how I described it in the previous paragraph, and then suggest that an Action subclass with an overridden __call__ is the most direct way to implement the Action API?

In reviewing the code again, I note that not just most, but all of the Action subclasses override __init__. I also note that not all of them accept the same parameters. For example, the _StoreConstAction does not accept an nargs parameter.

>>> p = argparse.ArgumentParser()
>>> p.add_argument('--awesome-sauce', action="store_const", const=True, nargs=None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "c:\python\lib\argparse.py", line 1281, in add_argument
    action = action_class(**kwargs)
TypeError: __init__() got an unexpected keyword argument 'nargs'

So it seems that the Action API is slightly different than I described. The first callable should accept all of the parameters passed to add_argument _except_ for the action parameter itself. I think by indicating this in the API description, we avoid the problem of freezing any additional signature.

I'll take another stab at updating the documentation with these findings.

[bethard]

Sorry about being out of contact (I'm flying back and forth between the US and the EU every 4-5 weeks right now), and thanks Terry for bringing this to my attention.

Essentially, the API is:

• The argument to action= must be a callable that accepts at least a "dest" and "option_strings" keyword arguments, and returns some object. The keyword arguments passed to this callable will be dest, option_strings, and whatever other keyword arguments were passed to add_argument().

• The object returned by the action= callable should have attributes "dest", "option_strings", "default", "type", "required", "help", etc. defined in essentially the same way as the add_argument() documentation.

• The object returned by the action= callable should itself be callable, and should accept the arguments (self, parser, namespace, values, option_string=None). This method can do whatever it wants, but as you note, the typical approach is to invoke setattr(namespace, self.dest, ...)

Now, all that said, the easiest way of creating a callable that returns an callable where both have the right signatures and the right attributes is to subclass argparse.Action. In fact, doing it any other way is probably crazy. I'm against changing the name of __call__ to invoke because it removes the possibility of defining an action as a function returning another function (which is currently possible), but I'm all for making the documentation strongly recommend subclassing argparse.Action. I would just say something like:

"You may also specify an arbitrary action by passing a subclass of :class:`argparse.Action`, or another callable object that implements the same interface."

I wouldn't bother to go into more detail about "implements the same interface" - all sane people will just subclass Action. ;-)

As to argparse.Action.__init__, hopefully the above bullet points make it clear that you must accept "dest" and "option_strings", but what other keyword arguments you want to accept are up to you. Be sure to call the superclass __init__ to set any attributes that you didn't accept to appropriate default values. A simple example of accepting an additional keyword argument is the _VersionAction, which accepts a "version" keyword argument that the other actions don't accept. The current "example of a custom action" should probably define __init__ to show how this works.

[BreamoreBoy]

@paul what is your opinion of this documentation patch?

[jaraco]

That's interesting that there's activity on this again. I had forgotten I'd filed it. It's particularly timely, as I just yesterday had this experience again. I looked at the documentation and found it inadequate for me to understand how to implement a custom action, so I went to the source to figure it out. The action I ended up creating was this one:

class SandboxAction(argparse.Action):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, nargs=0, **kwargs)

    def __call__(self, *args, **kwargs):
        cls.use()

Where 'cls' is defined in the same scope where SandboxAction was defined. I'm not sure if my documentation patch would have been sufficient to allow me to write that Action without consulting the source, but this example represents another case where that should be possible.

I haven't made any tweaks to the patch as a result of Steven's comments, so I probably could do that. I don't now when I'll have the time to do that, but I'll keep a tab to remind me to revisit the issue again.

[jaraco]

I've made another commit to capture Steven's recommendations. I've also merged the work with the latest 2.7 tip, the result of which is the latest diff. I'll commit this in a week or two if there aren't any objections, though I would appreciate a review and especially any suggestions on how I might test the correctness of the syntax.

[jaraco]

I've tried building the docs, but it appears to rely on a local build of Python as well as subversion, neither of which my system can do currently, so I'm hoping someone has a suitable environment to help test the building of the docs.