Skip to content

Improve subclass binding when __init__ is not defined. #17

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DBraun
Copy link

@DBraun DBraun commented Jul 30, 2024

I have a lot of subclasses of an abstract base class. The subclasses all use the the base's __init__, and so I don't explicitly write out the __init__ for each. However, this is causing an issue in argbind when I want to configure the kwargs in the init of each subclass differently. It ends up not configuring the kwargs at all and leaves them at their defaults.

Example of the current argbind:

import abc
from typing import Any, Dict

import argbind


class Base(abc.ABC):
  
    def __init__(self, config: Dict[str, Any] = None):
        self.config = config

    @abc.abstractmethod
    def action(self):
        """Do something"""


@argbind.bind()
class Sub1(Base):

    def action(self):
        print('hello', self.config)


@argbind.bind()
class Sub2(Base):

    def action(self):
        print('goodbye', self.config)


if __name__ == "__main__":

    argbind.parse_args() # add for help text, though it isn't used here.

    args = {
      'Sub1.config': {"a": "apple", "b": "banana"},
      'Sub2.config': {"c": "cherry", "d": "durian"},
    }

    with argbind.scope(args):
        Sub1().action() # prints "hello None"
        Sub2().action() # prints "goodbye None"

Notice that it didn't pass the requested config and instead left it as None.

With this PR, the last section is instead:

    with argbind.scope(args):
        Sub1().action() # prints "hello {'a': 'apple', 'b': 'banana'}"
        Sub2().action() # prints "goodbye {'c': 'cherry', 'd': 'durian'}"

I also updated examples/bind_existing/with_argbind.py to make more sense to me. It produces the same output as before, but I changed some code. I think it's confusing to put the superclass MyClass in the args/yaml. To me, it makes more sense to be able to put the subclass BoundClass.

@DBraun
Copy link
Author

DBraun commented Dec 5, 2024

@pseeth @seethlord can you take a look?

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

Successfully merging this pull request may close these issues.

1 participant