Union of singular type with list of types not behaving as expected.

[rwightman]

Describe the bug So this may not be a bug, but it's not clear what the expected behaviour is in any case. For class fields that are often singular but sometimes a list or tuple of values of the same type I often use Union[type, List[type]] in annotations. When it's a single value I access obj.value instead of obj.value[0], there is code to check for sequences and handle appropriately.

I thought this might just work with SimpleParsing, it accepts the type definition but does not behave the way I was hoping, and I cannot determine what it is supposed to do in this case, it looks like it ignores the list/tuple typing and behaves as if the field was just type

To Reproduce

from simple_parsing import ArgumentParser
from dataclasses import dataclass

@dataclass
class Foo:
   bar: Union[str, List[str]]

if __name__ == "__main__":
   parser = ArgumentParser()
   parser.add_arguments(Foo, "foo")
   args = parser.parse_args()
   foo: Foo = args.foo
   print(foo)

Expected behavior A clear and concise description of what you expected to happen.

$ python issue.py --bar alpha
Foo(bar='alpha')

$ python issue.py --bar alpha beta
Foo(bar=['alpha', 'beta'])

Actual behavior A clear and concise description of what is happening.

$ python issue.py --bar alpha
Foo(bar='alpha')

$ python issue.py --bar alpha beta
error: unrecognized arguments: beta

Desktop (please complete the following information):

• Version: 0.1.4
• Python version: 3.11

Additional context Add any other context about the problem here.

[lebrice]

Hey there @rwightman , thanks for posting!

Hmmm this is clearly a bug. I'm curious though, does this happen only with strings? Or do other field types exhibit the same behaviour?

[rwightman]

@lebrice it behaves similarly if I change str -> int in this example

[grahamannett]

found this as well and dove into it a bit more (from an earlier comment I deleted).

Seems like the issue is that for types.UnionType the utils._mro will return [] and probably utils._mro needs to use self.type.__args__ when its a UnionType? This causes the wrapped_field.arg_options to not have nargs='*' elsewhere which is needed for the original ArgumentParser bits.

I can fix and PR later but want to check how FieldWrapper works more. Seems like it might not actually be validating the inner annotation but I haven't checked fully.

[lebrice]

Thanks for looking into this @grahamannett, that's very helpful! The codebase is in need of a proper refactoring, sorry if it's not super clear what each thing does, but it's essentially this:

• DataclassWrapper: Stores state about where a given dataclass is added, and produces the arguments that are passed to parser.add_argument_group. Has a FieldWrapper for each field in the dataclass for which we should add command-line arguments.
• FieldWrapper: Stores some state about a field (where it is placed, what dataclass is its parent, what options were set in the field function, etc, and produces the arguments that are passed to parser.add_argument. In order to do that, it does some introspection on the type annotation and comments of the dataclass field, in order to produce the nargs, type, default, etc that are passed to argparse.

Hope this helps, lmk if you have other questions :)

[grahamannett]

Yeah thanks thats helpful @lebrice.

I can add a test and PR as I think the fix isn't super complicated. Just probably have to check what other expected functionality is for UnionType/List args which requires me to look at the FieldWrapper a bit more.

[grahamannett]

Ended up being more confusing/harder than I originally thought but should fix the issue.