Printing default values as well as help from the command line

[akinwilson]

I am trying to use the ArgumentDefaultsHelpFormatter class from the original argparse library to enhance the displayed help information when using the ArgumentParser class from the argparse_dataclass library. Suppose I run the following script.py with the help - -h - flag

from dataclasses import dataclass, field 
from argparse_dataclass import ArgumentParser
from argparse import ArgumentDefaultsHelpFormatter

@dataclass
class Args:
    val_a : int = field(default=30522, metadata={'help':'info about val_a'})
    val_b :int = field(default=768, metadata={'help':'info about val_b'})
    val_c : str = field(default='foo', metadata={'help':'info about val_c'})
    

if __name__ == "__main__":
    parser =ArgumentParser(Args,formatter_class=ArgumentDefaultsHelpFormatter)
    args = parser.parse_args()

I would expect the terminal output to be like:

usage: script.py [-h] [--val-a VAL_A ] [--val-b VAL_B] [--val-c VAL_C]

options:
  -h, --help            show this help message and exit
  --val-a VAL_A
                        info about val_a (default:30522)
  --val-b VAL_B 
                        info about val_b (default: 768)
  --val-c VAL_C 
                        info about val_c (default: foo)

but its actually like:

usage: script.py [-h] [--val-a VAL_A ] [--val-b VAL_B] [--val-c VAL_C]

options:
  -h, --help            show this help message and exit
  --val-a VAL_A
                        info about val_a (default: <dataclasses._MISSING_TYPE object at 0x711c99aeed70>)
  --val-b VAL_B 
                        info about val_b (default: <dataclasses._MISSING_TYPE object at 0x711c99aeed70>)
  --val-c VAL_C 
                        info about val_c (default: <dataclasses._MISSING_TYPE object at 0x711c99aeed70>)

Does anyone know how to rectify this behavior?

EDIT
As I am seeking help from multiple places on the subject, the other being a stack overflow post, it was pointed out that this behavior is due to these lines in the source code

could potentially editing these lines in the source code from

def _add_dataclass_options(
    options_class: Type[OptionsType], parser: argparse.ArgumentParser
) -> None:
    if not is_dataclass(options_class):
        raise TypeError("cls must be a dataclass")

    for field in fields(options_class):
        args = field.metadata.get("args", [f"--{_get_arg_name(field)}"])
        positional = not args[0].startswith("-")
        kwargs = {
            "type": field.metadata.get("type", field.type),
            "help": field.metadata.get("help", None),
        }

to

def _add_dataclass_options(
    options_class: Type[OptionsType], parser: argparse.ArgumentParser
) -> None:
    if not is_dataclass(options_class):
        raise TypeError("cls must be a dataclass")

    for field in fields(options_class):
        args = field.metadata.get("args", [f"--{_get_arg_name(field)}"])
        positional = not args[0].startswith("-")
        kwargs = {
            "type": field.metadata.get("type", field.type),
            "help": field.metadata.get("help", None),
            "default": field.default,
        }

stop this behavior from happening?

[akinwilson]

I see from here that this cannot apparently be changed due to the consequences it has on the default_factory argument.

But after having created a fork and testing it out with the changes above and removing the assignment of default key value to MISSING later in the code, it prints the help information along with default values too as expected.

What exactly are the consequences of not using the sentinel class value of MISSING from the dataclasses library for the default value?

[jcal-15]

The thought behind having the dataclass handle the default value was to make sure that creating an instance of the dataclass outside of the argparse_dataclass package behaved the same as when created using the package.

Maybe the following example will be helpful in explaining what I feel needs to be avoided. In older versions this package, the default_factory function was being called when adding arguments to the ArgumentParser instance. This meant that default_factory was always being called even when it was not needed. Also, if default_factory was needed to be called and parse_args was called more than once, the resulting data-class instances would have the same value for that field (breaking the purpose of default_factory).

That being said, I am not able to think of a use case where having the argparse_dataclass use field.default would break anything. I think originally I did not do this because I was worried that more advanced features like __post_init__ might not work the same and chose to play it safe.

I have a draft PR open if you want to review and see if this change works for you. I also would like to think about it a few more days to be sure I am not missing something and obviously get mivade's opinion. It is PR #66

[akinwilson]

I'm with you. I'm going to be honest, I'm not too sure what the purpose of the default_factory is (in this application) as long as you plan to provide a default value, it's purpose subsides right?

I too would be worried about additional methods like post_init causing unexpected behaviour.

It's not directly related to this specific issue, but I was actually researching the behaviour of dataclasses when using inheritance, in preparation for using it with this library. you can find the discussion here it may be helpful with illuminating the behaviour you're concerned about.

[mivade]

I've not ever actually used help formatters with argparse. Is there any reason we couldn't just provide our own that does this?

[akinwilson]

This was my first time using it too. I was pleasantly surprised at how closely it worked straight out the box with this library.

Defining one specific to this library could be done, but I personally would probably opt for the easier option and superficially I thought it was the above change but it may not be if you don't want to constrain people to using the argparse library in specific ways like I'm getting the impression it would do with my suggestion.

[cdleonard]

The real problem with calling default_factory at parser creation time is that multiple calls to parse_args will share the same value. This happens because argparse will internally assign the default value to fields when parsing.

It's possible to avoid this by passing a namespace into argparse which already contains a fresh value for any field that has a default or default_factory defined at the dataclass level.

This solution (posted as PR #70) ensures that each parse_args call gets a fresh value from default_factory - which is the desired behavior. The only downside is that it can call default_factory more time than strictly needed - once at parser creation time and again at parse time. This would only be a problem if your default_factory implementation has side effects which would be a horrible design. Some unit tests do this on purpose and I had to relax them. Allowing multiple calls to default_factory seems very reasonable because most people just use default_factory=list.

An alternative solution would be to write a custom HelpFormatter for argparse_dataclass which is aware of dataclasses and will call default_factory() only at --help print time.