-
Notifications
You must be signed in to change notification settings - Fork 15
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
Remove restrictions on inheriting partial'd configs #290
Conversation
@jgbos would you mind reviewing this / working off this branch during your day-to-day (if you aren't already) to confirm that you don't hit bugs? I still have some lingering to-dos, but they are relatively minor compared to the rest of this PR |
@rsokl fyi, I haven't been running into any issues with the branch all week (I haven't done a lot though). |
@jgbos great! I plan to try to tackle the outstanding To-Dos when possible and then merge this |
There are restrictions on inheritance patterns permitted by
builds
. Specifically, one could not inherit from aPartialBuilds
-type config without explicitly "opting-in" and specifyingzen_partial=True
on the child. And even in this latter case, one could not include zen-processing features (such aszen_meta
) in the child config.See #289 for a detailed discussion of these restrictions.
This PR lifts all of these restrictions!
Before:
The default value for
zen_partial
wasFalse
. There was no way for a user to inherit from a partial-builds config and produce a builds-configAfter:
The default value for
zen_partial
is nowNone
. Specifyingzen_partial
explicitly will determine whether the resulting config is a partial-builds, otherwise it is inherited or defaults toFalse
It achieves this by changing the default value of
builds(..., zen_partial)
to beNone
instead ofFalse
.None
indicates that the config will not entail partial instantiation unless that behavior is inheritedFalse
indicates will not entail partial instantiation under any circumstancesmake_config
will now also raise when the following patterns are detected in a config:_zen_target
is specified but_target_
does not point tohydra_zen.funcs.zen_processing
. This indicates a multiple inheritance pattern that "clobbers" the zen-processing configuration pattern_partial_=True
is specified in conjunction with zen-processing patternsTo-Do:
make_config
when inheriting_partial_=True
and_zen_partial=True
fields simultaneouslyis_partial
and/orvalidate_partial
so that users can inspect whether or not a config is a partial config without needing insights into the two different protocols for partial configs.Closes #289
Closes #288