-
Notifications
You must be signed in to change notification settings - Fork 54
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
Using subgroups with config_path
leads to a TypeError
#276
Comments
Hey there @anivegesana , thanks for posting! I'll have to look into this bug more closely, but this seems to be related to these lines here: https://github.com/lebrice/SimpleParsing/blob/master/simple_parsing/helpers/serialization/serializable.py#L793-L799. Just to check, was this config created with the
I'll try to look into it, thanks again for posting! |
Hey @lebrice, thanks for looking into this! I actually didn't know whether to file this as a feature request or as a bug report since it is a little bit of both, and I think I might have confused you with an incomplete description of what I wanted/expected. Parsing of subgroups is working perfectly for me and loading configs from external files is working perfectly for me as well. The problem is that if I load a configuration that uses subgroups from an external file, with or without Traceback (most recent call last):
File "run.py", line 39, in <module>
args = parser.parse_args()
File "/home/anirudhvegesana/anaconda3/envs/py38/lib/python3.8/argparse.py", line 1768, in parse_args
args, argv = self.parse_known_args(args, namespace)
File "/home/anirudhvegesana/anaconda3/envs/py38/lib/python3.8/site-packages/simple_parsing/parsing.py", line 333, in parse_known_args
self._preprocessing(args=args, namespace=namespace)
File "/home/anirudhvegesana/anaconda3/envs/py38/lib/python3.8/site-packages/simple_parsing/parsing.py", line 523, in _preprocessing
wrapped_dataclasses, chosen_subgroups = self._resolve_subgroups(
File "/home/anirudhvegesana/anaconda3/envs/py38/lib/python3.8/site-packages/simple_parsing/parsing.py", line 651, in _resolve_subgroups
assert argument_options["default"] is subgroup_field.subgroup_default
AssertionError In either case, there is no way is specify which subgroup to choose from the config file. This part is more of a feature request. I am fine with if the subgroup is chosen via the |
Since the subgroup selection is not stored directly, I don't expect it to be easy to save the subgroups to configuration files via I can open separate issue for subgroup selection via file if these two ideas turn out to be very different from each other, or if you have other ideas about how to accomplish this. |
Signed-off-by: Fabrice Normandin <[email protected]>
Signed-off-by: Fabrice Normandin <[email protected]>
Thanks a lot @anivegesana , your explanation really helped me to better undestand the issue. I've been playing around with this a bit, and I think it's more of a bug than a feature request. I don't think that the loading of subgroups from config files is working correctly. I'll hopefully keep you posted on this, please do ping me here if I don't reply soon-ish (within say a week) |
Hey @lebrice, You told me to check in within a week. How is it going and how far along have you gotten in figuring out why |
Hey @anivegesana , thanks for checking in! I've gotten one part of it to work in #284, the As for the rest of it (the AssertionError and TypeError you mention here), I'm not done exploring the issue yet, but it seems to be caused by the assumptions I had when I implemented the subgroups feature:
The issue seems to be with the first assumption, because I'm assuming that the field is first a simple I'm not done digging on this, but here's a little overview of what I've found so far. |
Ok. Good to hear that you are making progress on the issue! Hopefully the other half of the problem isn't that challenging. |
@lebrice Hi, is there any update ? |
|
Hey @lebrice @anivegesana - is this finally fixed? This would be a much needed improvement for me too. |
|
Describe the bug
When using subgroups with
config_path
, a type error occurs when trying to resolve which subgroup the config file should belong to. Adding a_type_
or subgroup key attribute automatically to all such entries and instantiating dictionaries that are assigned to subgroups inside of config files is a potential fix.To Reproduce
Expected behavior
The subgroup entries to be read into the
ArgumentParser
and able to be modified via command line.Actual behavior
A clear and concise description of what is happening.
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: