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

Could a subclass type hint be a config file when using CLI? #57

Closed
MendelXu opened this issue May 20, 2021 · 8 comments
Closed

Could a subclass type hint be a config file when using CLI? #57

MendelXu opened this issue May 20, 2021 · 8 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@MendelXu
Copy link

MendelXu commented May 20, 2021

I want to load a config file with the structure like:

input:
  class_path: src.input.KeyBoard
  init_args: input/keyboard.yaml

But it would not parse the file specified in init_args as a Dict. I also try to write it as

input:
  class_path: src.input.KeyBoard
  init_args:
    cfg: input/keyboard.yaml

and it still does not work. Is there any way to implement such function?

@mauvilsa
Copy link
Member

Could you please add information about how you are creating the parser. Would be even better if it is a small code snippet that can be easily run.

Right now I am not sure if this can be easily implemented. Though in general I think this is not a good idea. The class_path and init_args jointly specify a subclass. In other words what init_args can be depends on the value of class_path. Having these two in separate files can make things confusing. It would advice to have instead:

# main.yaml
input: input/keyboard.yaml
# keyboard.yaml
class_path: src.input.KeyBoard
init_args:
  arg1: val1
  ...

Or maybe what you want is have a configurable class (not a subclass), so KeyBoard is fixed and in a separate file have its args. In this case having the args in a separate file would make sense.

@mauvilsa mauvilsa added the question Further information is requested label May 20, 2021
@MendelXu
Copy link
Author

MendelXu commented May 20, 2021

Thanks for your replying. I create a small case like

#main.py
from jsonargparse import CLI


class B:
    def __init__(self, meta_info: dict):
        self.meta_info = meta_info


class A:
    def __init__(self, b: B = None, a_lot_of_params: dict=None):
        self.b = b
        self.a_lot_of_params = a_lot_of_params
        
    def hello(self):
        print('I am ok')

if __name__ == "__main__":
    CLI(A)
#cfg.yaml
b: b.yaml
a_lot_of_params: 
  a: 1
  b: 2
#b.yaml
class_path: __main__.B
init_args: 
  meta_info:
    something: true
    others: 1

By running python main.py --config cfg.yaml hello, I want to instantiate an object of class B for the class A.
However, it would return the error that

error: Parser key "b": Value "b.yaml" does not validate against any of the types in typing.Union[__main__.B, NoneType] :: Problem with given class_path "b.yaml" :: No module named 'b' :: Expected a <class 'NoneType'> but got "b.yaml"

@mauvilsa
Copy link
Member

If you change line signatures.py#L295 to kwargs['action'] = ActionTypeHint(typehint=annotation, enable_path=True) it will work as you want. The unit tests of the package don't break with this change. But I should think a bit if this could result in some unexpected consequences. If not this could be added as a parameter of CLI to enable it or maybe it would make sense to be enabled by default.

@mauvilsa mauvilsa added the enhancement New feature or request label May 20, 2021
@mauvilsa mauvilsa changed the title Could init_args be a config file? Could a subclass type hint be a config file when using CLI? May 20, 2021
@MendelXu
Copy link
Author

Got it. Thank you very much!

@MendelXu
Copy link
Author

MendelXu commented May 20, 2021

OK. I find that in a more complicated situation, it still fails.

#main.py
from jsonargparse import CLI

class B:
    def __init__(self, meta_info: dict):
        self.meta_info = meta_info


class A:
    def __init__(self, b: B = None, a_lot_of_params: dict=None):
        self.b = b
        self.a_lot_of_params = a_lot_of_params
        
    def hello(self):
        print('I am ok')

class C(A):
    def __init__(self,b: B = None, a_lot_of_params: dict=None):
        pass

if __name__ == "__main__":
    CLI(A)
#cfg.yaml
class_path: C
init_args:
  b: b.yaml

And the error is

error: Conflicting namespace base: init_args.b, expected dict but is b.yaml.

@MendelXu MendelXu reopened this May 20, 2021
@mauvilsa
Copy link
Member

With the code snippet above I am unable to reproduce that error message. Furthermore, class components given to CLI do not behave like subclass type hints. The main config file can't start with a class_path and init_args pair. What is supported is that b could be a subclass of B.

mauvilsa added a commit that referenced this issue May 24, 2021
… CLI #57.

- With fail_untyped=True use type from default value instead of Any.
- Added ruyaml optional unit tests.
@mauvilsa
Copy link
Member

I am not sure what you are trying to achieve. If you want to be able to run A or C then maybe what you want might be CLI([A, C]) which would make each class a subcommand.

@MendelXu
Copy link
Author

Sorry for the late reply. I am on a deadline and unable to deal with it now. I think I can close the issue and process it after my deadline. Thank you very much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants