omni-us / jsonargparse

Implement minimal boilerplate CLIs derived from type hints and parse from command line, config files and environment variables
https://jsonargparse.readthedocs.io
MIT License
325 stars 49 forks source link

Feature Request: jsonargparse could accept omegaconf "MISSING" (???) -notation #219

Closed vedal closed 1 year ago

vedal commented 1 year ago

🚀 Feature request

Currently, jsonargparse does not seem to support the ??? notation of omegaconf, such as for example hydra. Example:

model:
  experiment_name: ???
from dataclasses import dataclass
from jsonargparse import CLI

@dataclass
class MyModel:
    experiment_name: str

def fit(model: MyModel):
    pass

if __name__ == '__main__':
    CLI(fit, parser_mode='omegaconf')

$ python example_jsonargparse4.py --config example_jsonargparse4.yaml --model.experiment_name foo

fails.

An option that currentyl works is to comment out the parameter from the yaml as:

model:
  # experiment_name: ???

... but this seems less intuitive for the user

Motivation

Allowing the "???" notation could be an intuitive way of creating"abstract" default configs, where the yaml itself show how some parameters be set before running the experiment.

Pitch

Should crash if config isnt set on the command line or in a follow-up yaml.

Alternatives

Not completely sure if this should be a feature, or if there are better ways of doing this. It could also be a misunderstanding from my side. However, I thought it might be useful to discuss.

mauvilsa commented 1 year ago

Thank you for the proposal! Indeed there is already a way to do this. Using your example above, one can do:

$ python cli.py --print_config
model:
  experiment_name: null
$ python cli.py --print_config > config.yaml
$ python cli.py --config config.yaml
usage: cli.py [-h] [--config CONFIG] [--print_config[=flags]] [--model CONFIG] --model.experiment_name EXPERIMENT_NAME
cli.py: error: Configuration check failed :: Key "model.experiment_name" is required but not included in config object or its value is None.
$ python cli.py --config config.yaml --model.experiment_name foo
$

I don't see a need for some special notation for this. null is native in yaml which in my opinion is better than some special string value.

mauvilsa commented 1 year ago

Also see https://github.com/Lightning-AI/lightning/issues/15109#issuecomment-1278522476

vedal commented 1 year ago

@mauvilsa thanks for your answer! I assumed that providing a "null" in yaml would be parsed as "None", which for hydra isnt the same as directly crashing the application. However, since jsonargparse requires that all parameters in yaml be defined with a type which is not None, otherwise error... ValueError: Required parameter without a type for "__main__.MyModel.__init__" parameter "parameter_name". ... I agree that passing null will crash the application (maybe there's some corner-case with Optional type, but I didn't find it.

mauvilsa commented 1 year ago

In version v4.19.0 just released, the CLI function now supports the fail_untyped parameter. If you give fail_untyped=False, required parameters will get as type Any and not fail with ValueError: Required parameter without a type .... Though, still it is recommended to add type hints to all the signatures that will be used with CLI.

vedal commented 1 year ago

Great, thanks for keeping me updated on this!