Skip to content

handle SCMode.INSTANTIATE with throw_on_missing=False#1104

Draft
Jasha10 wants to merge 1 commit intoomry:masterfrom
Jasha10:i1103
Draft

handle SCMode.INSTANTIATE with throw_on_missing=False#1104
Jasha10 wants to merge 1 commit intoomry:masterfrom
Jasha10:i1103

Conversation

@Jasha10
Copy link
Collaborator

@Jasha10 Jasha10 commented Jul 16, 2023

This PR enables OmegaConf.to_container(..., throw_on_missing=False) to work for structured configs when structured_config_mode=SCMode.INSTANTIATE. Closes #1103.

In addition, this PR moves MISSING from omegaconf.py into base.py to avoid a circular import issue.

Here is an example of the behavior enabled by this PR:

# repro.py
from dataclasses import dataclass
from enum import Enum
import omegaconf
from omegaconf import OmegaConf

@dataclass(frozen=True)
class A:
    x: int

a = OmegaConf.create(A)
container = OmegaConf.to_container(
    a,
    throw_on_missing=False,
    structured_config_mode=omegaconf.SCMode.INSTANTIATE,
)

assert isinstance(container, A)
print(container)

BEFORE:

$ python repro.py
Traceback (most recent call last):
  File "/home/homestar/dev/omegaconf/repro.py", line 12, in <module>
    cfg = OmegaConf.to_container(
...
omegaconf.errors.MissingMandatoryValue: Structured config of type `A` has missing mandatory value: x
    full_key: x
    object_type=A

AFTER:

$ python repro.py
A(x='???')

) -> Dict[str, Any]:
from omegaconf.omegaconf import MISSING, OmegaConf, _maybe_wrap
from omegaconf import MISSING, OmegaConf
from omegaconf.omegaconf import _maybe_wrap

Check notice

Code scanning / CodeQL

Cyclic import

Import of module [omegaconf.omegaconf](1) begins an import cycle.
type_str,
)
from .base import Box, Container, ContainerMetadata, DictKeyType, Node
from .base import MISSING, Box, Container, ContainerMetadata, DictKeyType, Node

Check notice

Code scanning / CodeQL

Cyclic import

Import of module [omegaconf.base](1) begins an import cycle.
),
)
else:
v = MISSING

Check warning

Code scanning / CodeQL

Variable defined multiple times

This assignment to 'v' is unnecessary as it is [redefined](1) before this value is used. This assignment to 'v' is unnecessary as it is [redefined](2) before this value is used.
@omry
Copy link
Owner

omry commented Jul 21, 2023

Looks good overall and I agree that it's a better behavior.
I think there is still some circular dependency.

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.

One should be allowed to re-instantiate a structured object with missing values

2 participants