lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
410 stars 52 forks source link

Dataclass arguments added after `parse_known_args` call don't get added to help #314

Closed pfrwilson closed 2 months ago

pfrwilson commented 3 months ago

Describe the bug A common pattern I used to use to add arguments that depend on a certain arguments, e.g. debug options that should only be added if a "--debug" flag is set, is to use args, _ = parse_known_args, check for the flag, then add more options via parser.add_argument(...). I set add_help=False when I create the parser, but I add it back in after all the arguments are added, so the user would see an expanded help if the flag is set.

With simple_parsing, i'd like to be do this, but using the parser.add_arguments(...) method. However, any arguments added after the args, _ = parse_known_args through the parser.add_arguments(...) method do not appear in the help message, even though they do appear in the parsed object

To Reproduce

from simple_parsing import ArgumentParser
from dataclasses import dataclass

@dataclass
class ModelConfig:
    hidden_size: int = 128
    num_layers: int = 2
    dropout: float = 0.1

@dataclass
class TrainConfig:
    batch_size: int = 32
    learning_rate: float = 1e-3
    num_epochs: int = 10

@dataclass
class DebugOptions:
    verbose: bool = False
    limit_batches: int = 100

parser = ArgumentParser(add_help=False)
parser.add_arguments(ModelConfig, dest="model")
parser.add_arguments(TrainConfig, dest="train")
parser.add_argument('--debug', action='store_true')
args, extras = parser.parse_known_args()
print(args.debug)
if args.debug:
    parser.add_arguments(DebugOptions, dest="debug_options")
parser.add_argument('--help', '-h', action='help', help='Show this help message and exit')

args = parser.parse_args()
print(args)

Expected behavior

$ python issue.py --debug -h 

should print the whole help, including the help for the DebugOptions class.

Actual behavior It only prints the following:

usage: scratch.py [--debug] [--hidden_size int] [--num_layers int] [--dropout float] [--batch_size int] [--learning_rate float] [--num_epochs int] [--help]

options:
  --debug
  --help, -h            Show this help message and exit

ModelConfig ['model']:
  ModelConfig(hidden_size: int = 128, num_layers: int = 2, dropout: float = 0.1)

  --hidden_size int     (default: 128)
  --num_layers int      (default: 2)
  --dropout float       (default: 0.1)

TrainConfig ['train']:
  TrainConfig(batch_size: int = 32, learning_rate: float = 0.001, num_epochs: int = 10)

  --batch_size int      (default: 32)
  --learning_rate float
                        (default: 0.001)
  --num_epochs int      (default: 10)

Desktop (please complete the following information):

Additional context I know it would be possible to do this same behavior using subgroups, and in some ways subgroups are better (especially for providing typed configurations, etc.) but in the interest of keeping maximum compatibility with the argparse library, it may be worth fixing this bug.

lebrice commented 3 months ago

Hey there, thanks for posting!

Yeah this indeed seems like a bug. In the meantime, if you re-created the parser (with all the same arguments) and used it the second time, you should have somethig that works. For example:

def get_parser(with_debug: bool = False, add_help: bool = False):
    parser = ArgumentParser(add_help=add_help)
    parser.add_arguments(ModelConfig, dest="model")
    parser.add_arguments(TrainConfig, dest="train")
    parser.add_argument('--debug', action='store_true')
    if with_debug:
        parser.add_arguments(DebugOptions, dest="debug_options")
    return parser

first_parser = get_parser(with_debug=False, add_help=False)
args, extras = first_parser.parse_known_args()
if args.debug:
    second_parser = get_parser(with_debug=True, add_help=True)
    args = second_parser.parse_args(extras)    

I've coded this up quickly just to give you the idea, this might not be exactly what you want though.

pfrwilson commented 2 months ago

Hi @lebrice,

Thanks for your response and sorry for the delay. Your code snippet works - I'll close the issue as my use case is already supported natively by subgroups and the above code snippet is a good workaround. Thanks again!