omni-us / jsonargparse

Implement minimal boilerplate CLIs derived from type hints and parse from command line, config files and environment variables
https://jsonargparse.readthedocs.io
MIT License
311 stars 42 forks source link

CLI with non primitive types + subcommands #430

Open AlejandroBaron opened 7 months ago

AlejandroBaron commented 7 months ago

Hello!

I'm trying to do combine the CLI + config utility with subcommands and i'm not able to.

The CLI takes an object as an argument and in the subcommand it calls an object submethod.

This is a minimal example with 3 files

python example.py --config config.yaml subcommand

myclass.py

class MyClass:

    def __init__(self, x: int):
        self.x = x

    def method(self):
        print(self.x*2)

example.py (entrypoint)

from jsonargparse import CLI
from myclass import MyClass

class MyCLI:

    def __init__(self, myobj: MyClass):

        self.myobj = myobj

    def subcommand(self):
        self.myobj.method()

if __name__ == "__main__":
    CLI(MyCLI)

config.yaml

myobj:
  class_path: myclass.MyClass
  init_args:
    x: 2

Is this not supported yet?

mauvilsa commented 7 months ago

In principle it should be supported. But I have never tried having a subclass type in an __init__ parameter a class given to jsonargparse.CLI. Could be that there is a problem with it.

mauvilsa commented 7 months ago

The problem is that the definition myobj: MyClass is interpreted as required/positional. When parsing the arguments of python example.py --config config.yaml subcommand after parsing the config (optional), it then tries to parse myobj as a positional, but it finds subcommand (the first given positional), which obviously is not a valid MyClass. I know that inside config.yaml there is already a valid value for myobj, but this is independent of parsing of positionals. I am not sure how this could be fixed or if it is even possible.

@AlejandroBaron changing myobj to be non-positional would make it work. Just change the definition to myobj: Optional[MyClass] = None or add a default like myobj: MyClass = lazy_instance(MyClass, x=1). Also, you need to change the name of the method subcommand since it conflicts with the key used to know which subcommand was selected. It gives non-understandable message, so I need to fix that.

mauvilsa commented 6 months ago

With pull request #444 a better error message is given when a class has a subcommand method conflicting with the key where the chose subcommand is stored.