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
322 stars 47 forks source link

Fails to save arguments into a config file #440

Closed hadipash closed 8 months ago

hadipash commented 9 months ago

🐛 Bug report

To reproduce

from jsonargparse import ActionConfigFile, ArgumentParser

def do_something(args):
    save_path = "some/generated/path/"
    ArgumentParser().save(args, save_path + "test.yaml", format="yaml", skip_check=True)

if __name__ == "__main__":
    parser = ArgumentParser()
    parser.add_argument("--config", action=ActionConfigFile)
    parser.add_argument("--test_str", type=str, default="test")
    cfg = parser.parse_args(["--config=config.yaml", "--test_str=test_test"])
    do_something(cfg)

Output:

yaml.representer.RepresenterError: ('cannot represent an object', Path_fr(config.yaml, cwd=/home/projects/jsonargparse))

Expected behavior

It works if args.config[0] is explicitly converted into a string or config key is popped from args.

Or, is there a better way to handle these kinds of situations?

Environment

mauvilsa commented 9 months ago

save/dump are supposed to be used with the same ArgumentParser as the one used to parse. This is necessary so that it can be known how to serialize each key. That is why you get that error, since the empty parser ArgumentParser() doesn't know about the config key. Why is it that you want to do this?

hadipash commented 9 months ago

This is because args may change due to environmental setup or system requirements, so I would like to save the updated version.

mauvilsa commented 9 months ago

Sorry, I am not sure I understand. The details of a parser could depend on environment or other factors. But why aren't you able to use the proper parser to save instead of an empty one? If it is in different places of the codebase, you could have a function that creates the parser, and create one right before saving. But preferably use the same parser instance that was used to parse.

hadipash commented 8 months ago

It is useful, for example, when the directory in which to save the config file is not known until a later stage in the program's execution. I feel that carrying the original parser throughout the script for the sole purpose of saving is a bit of an overkill.

mauvilsa commented 8 months ago

Indeed carrying around a parser is not nice. But you can have a function that creates the parser. Then have a save function that creates a new parser same as the original one and then do the save.

It is by design that the same parser should be used to save. It is a bit unfortunate that I added the skip_check parameter since it can be misleading. Anyway, if you really want to save a namespace without what the save method is intended to do, you can simple do:

data = strip_meta(args).as_dict()

and then just dump it as yaml. There is a risk that some things might not work, but not necessarily in your case.

hadipash commented 8 months ago

Thanks for detailed reply!