Issue with argument linking to list of subclass objects

[odedbd]

I started using the (very cool) feature of linking an argument to a list of subclass objects, as shown in the documentation.

It seems that when the target argument is a list of ParentClass defined as a class property, I cannot set it simply as List[ParentClass]. If I do so, I get an exception. In order for the link to work as expected, I must define the argument as a union of both ParentClass and a list of ParentClass: Union[ParentClass, List[ParentClass]]

In my use case, a single object rather than a list doesn't make sense for the relevant class property, so I would prefer to only allow for the property to be a list.

Interestingly, this problem doesn't happen if I define the argument directly using parser.add_argument, I only get it when I use parser.add_class_arugments.

from typing import List, Union

from jsonargparse import ArgumentParser, dict_to_namespace


class MyClass(object):
    def __init__(self, simple_str: str = '', an_int: int = 0):
        self.simple_str = simple_str
        self.an_int = an_int

    def __repr__(self):
        return f'{self.simple_str}, {self.an_int}'


class MySubClass(MyClass):

    def __init__(self, str_list: List[str] = None, **kwargs):
        if str_list is None:
            str_list = []

        self.str_list = str_list
        super().__init__(**kwargs)

    def __repr__(self):
        return super().__repr__() + f', {self.str_list}'


class MainClass(object):
    def __init__(
            self,
            simple_str: str = '',
            str_list: List[str] = None,
            # my_class_list: List[MyClass] = None, # THIS GIVES ERROR
            my_class_list: Union[List[MyClass], MyClass] = None, # THIS WORKS FINE
    ):

        if str_list is None:
            str_list = []
        if my_class_list is None:
            my_class_list = []

        self.simple_str = simple_str
        self.str_list = str_list
        self.my_class_list = my_class_list


if __name__ == '__main__':
    parser = ArgumentParser(exit_on_error=False)
    parser.add_class_arguments(MainClass)

    parser.link_arguments('simple_str', 'my_class_list.init_args.simple_str')
    parser.link_arguments('str_list', 'my_class_list.init_args.str_list')
    value = {
        "simple_str": "a simple string",
        "str_list": ["a", "b", "c"],
        "my_class_list": [
            {
                "class_path": "MyClass",
                "init_args": {

                }
            },
            {
                "class_path": "MySubClass",
                "init_args": {
                    "an_int": 1,
                }
            }
        ]
    }

    cfg = parser.parse_args(args=[], namespace=dict_to_namespace(value))
    init = parser.instantiate_classes(cfg)
    print(init.my_class_list)

Expected behavior

The argument linking should work when List[ParentClass] is defined as a property of a class.

Environment

• jsonargparse version (e.g., 4.8.0): 4.27.2
• Python version (e.g., 3.9): 3.8.5
• How jsonargparse was installed (e.g. pip install jsonargparse[all]): as part of pytorch lightning
• OS (e.g., Linux): Windows 11

[mauvilsa]

Thank you for reporting! This is fixed in #434.

[mauvilsa]

I have been searching online for uses of dict_to_namespace and the only ones I found are here and in #236. That is good in a sense because dict_to_namespace is just a leftover from an old jsonargparse version and kept to avoid breaking changes. But there isn't really a need for dict_to_namespace and using it can actually lead to confusion, because the generated nested namespaces might not be the same as a namespace returned by a parser. For example, if an argument has as type dict, then this would remain a dict. But dict_to_namespace converts every instance of a dict into a namespace.

I am trying to reduce the public API to avoid possibilities of people getting confused. As part of this dict_to_namespace is being deprecated in #797 and will be removed in v5. Just wanted to comment in places where dict_to_namespace is used to see if there is any real use case and if there are, decide what to do. In the example above I wouldn't consider dict_to_namespace necessary. The value can be created as a Namespace from the start.

[odedbd]

I agree that in this example dict_to_namespace isn't really necessary. As for #236, as I wrote in the my last comment there, we switched to using (nested) dataclasses and it had been working well for us.

Thanks for commenting about this change here, I do have one place where I used 'dict_to_namespace' as a convenience method, but I don't see any problem with replacing it with direct construction of a 'Namespace' instead.