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

Merge fails with pair of nested dataclasses #1003

Closed
Jasha10 opened this issue Sep 10, 2022 · 0 comments · Fixed by #1004
Closed

Merge fails with pair of nested dataclasses #1003

Jasha10 opened this issue Sep 10, 2022 · 0 comments · Fixed by #1004
Labels
bug Something isn't working structured config

Comments

@Jasha10
Copy link
Collaborator

Jasha10 commented Sep 10, 2022

This bug report is motivated by facebookresearch/hydra#2349 .

The below example demonstrates two merge operations involving nested dataclasses.
The first merge succeeds, the second one raises ConfigKeyError. I think raising here is undesirable.

from dataclasses import dataclass
from omegaconf import OmegaConf, MISSING

@dataclass
class X:
    d: int
    p: int

@dataclass
class Y(X):
    q: int

@dataclass
class A:
    x: X

@dataclass
class B(A):
    x: Y

OmegaConf.merge(  # succeeds
    A(x=X(d=10, p=2)),
    B(x=Y(d=MISSING, p=MISSING, q=10)),
)

OmegaConf.merge(  # raises ConfigKeyError: Key 'q' not in 'X'
    A(x=X(d=10, p=2)),
    B(x=MISSING),
)

I feel that if the first merge succeeds then the second one should succeed too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working structured config
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant