lebrice / SimpleParsing

Simple, Elegant, Typed Argument Parsing with argparse
MIT License
399 stars 49 forks source link

Fix nesting defaults (based on #113) #115

Closed lebrice closed 2 years ago

lebrice commented 2 years ago

Based on #113

iamgroot42 commented 2 years ago

@lebrice not sure if I'm doing something wrong, but the issues still seems to be there if a dataclass has two optional dataclasses, and both of them are provided in the default argument

iamgroot42 commented 2 years ago

@lebrice any idea why the build keeps failing on a specific Python version?

lebrice commented 2 years ago

I'll take a look @iamgroot42 , thanks! Do you have a little example for the two-dataclass case? That'll make it easier for me to fix it

iamgroot42 commented 2 years ago

@lebrice it's technically not a problem when I use SimpleParsing by itself. To give some context: the following code was intended to work in a way that would allow using a config file, but also provide CLI arguments that could override arguments read from the JSON (for e.g. majority of config params could be from the file, and experiment-specific ones could be provided via CLI). This is how I'd done it:

parser = ArgumentParser(add_help=False)
parser.add_argument("--config_file", help="Specify config file", type=Path)
args, remaining_argv = parser.parse_known_args()
# Attempt to extract as much information from config file as you can
config = TrainConfig.load(args.config_file, drop_extra_fields=False)
# Also give user the option to provide config values over CLI
parser = ArgumentParser(parents=[parser])
parser.add_arguments(TrainConfig, dest="train_config", default=config)
args = parser.parse_args(remaining_argv)

And here are the relevant structures:

@dataclass
class TrainConfig(Serializable):
    data_config: DatasetConfig
    epochs: int
    ...
    dp_config: Optional[DPTraining] = None
    adv_config: Optional[AdvTraining] = None
    ...
    cpu: Optional[bool] = False

@dataclass
class DatasetConfig(Serializable):
    name: str
    prop: str
    split: str = field(choices=["victim", "adv"])
    value: float
    drop_senstive_cols: Optional[bool] = False
    scale: Optional[float] = 1.0

@dataclass
class AdvTraining(Serializable):
    epsilon: float
    iters: int

@dataclass
class DPTraining(Serializable):
    epsilon: float
    delta: float
lebrice commented 2 years ago

Hey @iamgroot42 I've actually added a test here to try to reproduce your comment, but I'm unable to.. Seems to be fixed here. Can you take a look and let me know if this still doesn't work on your end? Thx!