-
Notifications
You must be signed in to change notification settings - Fork 110
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
One should be allowed to re-instantiate a structured object with missing values #1103
Comments
@amatsukawa what do you expect the output of |
Thanks for the response! Yes, that’s what I hope to get back.
I agree that it violates the type annotation. But we’ve already given up on
that in the initial construction of A() with MISSING as the default value,
before creating the structured DictConfig.
I wish to be able to convert to AND from template dataclasses with MISSING
fiends and structured DictConfigs. Right now I can only go in one direction
but I can’t come back.
…On Sat, Jul 15, 2023 at 3:37 AM Jasha Sommer-Simpson < ***@***.***> wrote:
@amatsukawa <https://github.com/amatsukawa> what do you expect the output
of OmegaConf.to_container to be in your example above? Do you want the
output to be equal to A("???")? Note that such output would violate the
type hint given in the definition of A.
—
Reply to this email directly, view it on GitHub
<#1103 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADRHQNEA2FUKIMEHZB7EXTXQJCE5ANCNFSM6AAAAAA2K2R3XE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Good point.
Thanks for identifying what changes are necessary. I've created draft PR #1104 which implements the change. @omry, if you get the chance, could you please give an opinion on whether this behavior change makes sense? |
Defining dataclasses to introduce schema seems like a great idea. But, sometimes we have missing values, and there is
omegaconf.MISSING
to express this. However:The use case is we may have partially filled objects that we want to manipulate, construct new configs out of, etc.
The problem is that this line forgets
.to_container
'sthrow_on_missing
argument:omegaconf/omegaconf/basecontainer.py
Line 289 in 7dae67e
and inside this function, it could be allowed here:
omegaconf/omegaconf/dictconfig.py
Line 741 in 7dae67e
It's currently hard for me logistically to contribute the simple PR, but hopefully I have identified the changes necessary sufficiently if the maintainers agree this should be supported.
The text was updated successfully, but these errors were encountered: