Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PrintConfigAction added twice #169

Closed
florianblume opened this issue Oct 5, 2022 · 5 comments
Closed

PrintConfigAction added twice #169

florianblume opened this issue Oct 5, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@florianblume
Copy link

🐛 Bug report

I use jsonargparse with Pytorch Lightning which crashes when I try to add another config argument in a class derived from PL's CLI. I added the new config argument like this

parser.add_argument('--slurm', action=ActionConfigFile)

and now it crashes on this line with the following error message

argparse.ArgumentError: argument --print_config: conflicting option string: --print_config

Modifying the line from

if isinstance(action, ActionConfigFile) and getattr(self, '_print_config', None) is not None:

to

if isinstance(action, ActionConfigFile) and getattr(self, '_print_config', None) is None:

(i.e. remove the not) solves this. Intuitively, I'd say this is what the line was supposed to be or am I mistaking something?

To reproduce

I tried to make a reproducible example without PL but it wouldn't work and I thought I first ask whether that's a mistake before spending further time creating one. If you need one, let me know, then I'll try again.

Expected behavior

The ActionConfigFile argument should be addable without the error to happen.

Environment

  • jsonargparse version: 4.13.0
  • Python version: 3.8.13
  • How jsonargparse was installed: pip install jsonargparse[signatures]
  • OS (e.g., Linux): CentOS Linux on the university's cluster but also happens on Ubuntu
@florianblume florianblume added the bug Something isn't working label Oct 5, 2022
@florianblume
Copy link
Author

Ok weird, on the cluster this solution actually doesn't work but on my local machine it does. I'll investigate this more, sorry that I opened the issue to early!

@mauvilsa
Copy link
Member

mauvilsa commented Oct 5, 2022

ActionConfigFile is not intended to be used more than once in a parser. Maybe it should give an error when someone tries to add a second one, and show a more useful message. What exactly is it that you are trying to achieve? Just guessing, but maybe what you want is:

parser.add_argument('--slurm', type=dict, enable_path=True)

@florianblume
Copy link
Author

Hey, thanks for your quick reply! I'm trying to achieve that I can pass the path to a configuration file and it is automatically parsed. So I do something like --slurm configs/slurm/v100s.yml. I didn't know you weren't supposed to use ActionConfigFile twice, it looked like you can use it that way in the docs. But maybe that was a misconception because PL adds options for --model and --data and I assumed there were also of type ActionConfigFile. I'll try what you suggested.

@mauvilsa
Copy link
Member

mauvilsa commented Oct 5, 2022

Adding an argument with type dict as I said would load from a config file. Though it would accept anything, i.e., no validation. If you want validation then you could use as type a dataclass. In lightning for the --model and --data cases the argument is added a bit differently, see cli.py#L123-L129. But internally uses the same action as the enable_path=True, which is different to ActionConfigFile.

Indeed this is not explained well in the docs.

@florianblume
Copy link
Author

Ok thanks got it! It works now.

mauvilsa added a commit that referenced this issue Oct 27, 2022
…o use default_config_files pytorch-lightning#15174.

- Trying to add a second config argument to a single parser raises an exception #169.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants