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

AST resolver - support super with arbitrary parameters #153

Closed
t-mesq opened this issue Jul 27, 2022 · 3 comments
Closed

AST resolver - support super with arbitrary parameters #153

t-mesq opened this issue Jul 27, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@t-mesq
Copy link

t-mesq commented Jul 27, 2022

🚀 Feature request

Currently (v4.12.0) the AST resolver fails when resolving calls to super that don't follow the immediate MRO (i.e., super(SubClass, self) for class SubSubClass(SubClass)). This used to be possible in <=v4.9.0, so would it be possible for AST resolver to handle these cases?

To reproduce

from jsonargparse import ArgumentParser

class BaseClass:
    def __init__(self, base_arg: int, *args, base_kwarg: str="base", **kwargs):
        self.base_arg = base_arg
        self.base_kwarg = base_kwarg

class SubClass(BaseClass):
    def __init__(self, *args, sub_kwarg: str="sub", **kwargs):
        super().__init__(*args, **kwargs)
        self.sub_kwarg = sub_kwarg

class SubSubClass(SubClass):
    def __init__(self, *args, subsub_kwarg: str="subsub", **kwargs):
        super(SubClass, self).__init__(*args, **kwargs). # <- 
        self.subsub_kwarg = subsub_kwarg

config = {"class_path": "SubSubClass", "init_args": {"base_arg": 3}}
parser = ArgumentParser()
parser.add_argument('--myclass', type=BaseClass)
print(parser.parse_args(["--myclass", str(config)]))

Running the script in v4.9.0 results in:

Namespace(myclass=Namespace(class_path='__main__.SubSubClass', init_args=Namespace(base_arg=3, base_kwarg='base', sub_kwarg='sub', subsub_kwarg='subsub')))

which is a valid initialisation for SubSubClass, though sub_kwarg isn't necessary.
In v4.12.0 (with JSONARGPARSE_DEBUG enabled), however, it results in an error:

...
2022-07-27 11:59:33,895 - ArgumentParser - DEBUG - AST resolver: super with arbitrary parameters not supported: Call(func=Attribute(value=Call(func=Name(id='super', ctx=Load()), args=[Name(id='SubClass', ctx=Load()), Name(id='self', ctx=Load())], keywords=[]), attr='__init__', ctx=Load()), args=[Starred(value=Name(id='args', ctx=Load()), ctx=Load())], keywords=[keyword(arg=None, value=Name(id='kwargs', ctx=Load()))])
2022-07-27 11:59:33,902 - ArgumentParser - ERROR - 'Configuration check failed :: No action for destination key "base_arg" to check its value.'
2022-07-27 11:59:33,906 - ArgumentParser - ERROR - Parser key "myclass": Problem with given class_path "__main__.SubSubClass":
...

Expected behavior

If possible, I would expect the previous script to produce:

Namespace(myclass=Namespace(class_path='__main__.SubSubClass', init_args=Namespace(base_arg=3, base_kwarg='base', subsub_kwarg='subsub')))

Environment

  • jsonargparse version: 4.12.0
  • Python version: 3.9
  • How jsonargparse was installed: pip install jsonargparse
  • OS: Linux

Thank you in advance!

@t-mesq t-mesq added the bug Something isn't working label Jul 27, 2022
@mauvilsa
Copy link
Member

Thank you for reporting. In v4.9.0 the fact that sub_kwarg is in the namespace is not just about being unnecessary. If BaseClass didn't have **kwargs, it would fail on instantiation, i.e.

>>> cfg = parser.parse_args(["--myclass", str(config)])
>>> parser.instantiate_classes(cfg)
...
TypeError: __init__() got an unexpected keyword argument 'sub_kwarg'

Super with arbitrary parameters wasn't supported in v4.9.0 and it isn't supported in v4.12.0. This isn't really a bug, but a feature request. Changing the labels accordingly.

@mauvilsa mauvilsa added enhancement New feature or request and removed bug Something isn't working labels Jul 28, 2022
@mauvilsa
Copy link
Member

mauvilsa commented Aug 1, 2022

This new feature has been implemented in commit 1751fe7.

@mauvilsa mauvilsa closed this as completed Aug 1, 2022
@t-mesq
Copy link
Author

t-mesq commented Aug 3, 2022

Thank you for getting on this so quickly!

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

No branches or pull requests

2 participants