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

Providing additional init_args in later config overwrites existing namespace instead of updating #142

Closed
florianblume opened this issue Jun 28, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@florianblume
Copy link

🐛 Bug report

With the new version 4.10.0, providing only init_args in a yaml config for a subclass is possible when class_path is set in some other config. Unfortunately, when I pass a model config using --model and then in a later config using --config additional init_args like so:

model:
    init_args:
        batch_size: 32

then this dictionary overwrites the existing, previously loaded model config completely. This means that there's not class_path entry anymore and the CLI crashes.

I tracked it down to the update function of class Namespace:

def update(self, value: Union['Namespace', Any], key: Optional[str] = None, only_unset: bool = False) -> 'Namespace':
    """Sets or replaces all items from the given nested namespace.

    Args:
        value: A namespace to update multiple values or other type to set in a single key.
        key: Branch key where to set the value. Required if value is not namespace.
        only_unset: Whether to only set the value if not set in namespace.
    """
    if not isinstance(value, Namespace):
        if not key:
            raise KeyError('Key is required if value not a Namespace.')
        if not only_unset or key not in self:
            self[key] = value
    else:
        prefix = key+'.' if key else ''
        for key, val in value.items():
            if not only_unset or prefix+key not in self:
                self[prefix+key] = val
    return self

value is not a Namespace in case of the update dict coming from the second config which then goes into the first part of the if statment: if not isinstance(value, Namespace):. This is the part where it simply overwrites the existing and properly loaded namespace of the model.

To reproduce

from jsonargparse import ActionConfigFile, ArgumentParser
import yaml

class ModelBaseClass():
    def __init__(self, batch_size: int=0):
        self.batch_size = batch_size

class ModelClass(ModelBaseClass):
    def __init__(self, image_size: int=0, **kwargs):
        super().__init__(**kwargs)
        self.image_size = image_size

parser = ArgumentParser(
    prog='app',
    description='Description for my app.'
)

parser.add_subclass_arguments(ModelBaseClass, 'model')
parser.add_argument('--config', action=ActionConfigFile) 

model = yaml.safe_dump({
    'class_path': 'ModelClass',
    'init_args': {
        'image_size': 10
    }
})

config = yaml.safe_dump({
    'model': {
        'init_args': {
            'batch_size': 5
        }
    }
})

parser.parse_args([f'--model={model}', f'--config={config}'])

Output:
app: error: Configuration check failed :: Parser key "model": Type <class 'model.ModelBaseClass'> expects: a class path (str); or a dict with a class_path entry; or a dict with init_args (if class path given previously). Got "{'init_args': {'batch_size': 5}}".

@florianblume florianblume added the bug Something isn't working label Jun 28, 2022
mauvilsa added a commit that referenced this issue Jun 29, 2022
@mauvilsa
Copy link
Member

Fixed in commit c4ea888.

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

No branches or pull requests

2 participants