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

Issue with argument linking to list of subclass objects #433

Closed
odedbd opened this issue Jan 22, 2024 · 1 comment · Fixed by #434
Closed

Issue with argument linking to list of subclass objects #433

odedbd opened this issue Jan 22, 2024 · 1 comment · Fixed by #434
Labels
bug Something isn't working

Comments

@odedbd
Copy link

odedbd commented Jan 22, 2024

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
Copy link
Member

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

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

Successfully merging a pull request may close this issue.

2 participants