-
Notifications
You must be signed in to change notification settings - Fork 11
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
New Feature: zen_wrappers #117
Conversation
# flip order: first wrapper listed should be called first | ||
wrappers = tuple( | ||
get_obj(path=z) if isinstance(z, str) else z for z in _zen_wrappers | ||
)[::-1] | ||
|
||
obj = get_obj(path=_zen_target) | ||
|
||
for wrapper in wrappers: | ||
obj = wrapper(obj) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your comment "first wrapper listed should be called first" does not seem to agree with the result:
>>> obj = 123
>>> def first(x):
... print("first")
... return x
...
>>> def second(x):
... print("second")
... return x
...
>>> _zen_wrappers = [first, second]
>>> wrappers = tuple(
... get_obj(path=z) if isinstance(z, str) else z for z in _zen_wrappers
... )[::-1]
>>> for wrapper in wrappers:
... obj = wrapper(obj)
...
second
first
>>> # the second wrapper listed is called first
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yep! Good catch.
Saving some additional polish and validation, I am going to close this PR and open a series of smaller ones:
After this, I want to implement a nice interface for customizing the defaults on Please feel free to continue to leave feedback on this PR, to keep the thread of discussion in one place. |
I'm pretty excited about this.. 😄
New Feature: zen_wrappers
The introduction of
hydra_zen.funcs.zen_processing
opened the door to incredible power and flexibility for modifying the instantiation process.zen_wrappers
allows the user to "inject" one or more wrappers that will wrap the target-object before it is instantiated.Whereas
ultimately calls
Using
zen_wrappers
asultimately calls
A Basic Example
Let's cut to the chase and see this in action:
And the yaml is nice and readable
Incredibly... even the wrappers can be configured
It is kind of unbelievable how nicely this all fits together!
A Realistic Example: Data-Validation with pydantic!
Yeah... this totally works! Try it out by checking out this PR's branch!
This works really well... even with nested configs!
Why a Wrapper?
Wrapping gives the user access to the inputs, the target, and the outputs! This enables pre-processing, post-process, transformation, and ...circumvention? This means that we can expose a single interface that enables users to do.. anything! And the implementation on our end is super simple.
The pydantic implementation was already a great proving ground for this. See the implementation here. Their
validate_arguments
doesn't work on instance methods, but I was able to pretty easily (and elegantly!) work around that. This really drives home the flexibility of the wrapper paradigm.Enabling third parties to implement and provide their own wrappers
EDIT: This probably isn't necessary. Third party, or fourth party, sources can make these available / directly importable.
validates_with_pydantic
was pretty easy to implement, but ultimately it would be nice if third parties were responsible for their own implementations. This looks like a job for entry-points!It would be nice for hydra-zen to expose an entry-point. Suppose bear-type wants people to be able to use its type-validation abilities, but nothing in its public-API quite gets the trick done. He could implements some function
bear_type._internal.hydra_zen_validator
and then make it available to anyone who hashydra_zen
andbear_type
installed:and we would look for it with:
I'm not exactly sure where to go from there, in terms of making the various wrappers nice and discoverable/importable to users. But I am sure that this can be done.
Wrangling Unwieldy Configs
For validation with pydantic, users likely want to always use
hydra_convert="all"
so that pydantic doesn't get mad at omegaconf containers. But this makes for really clunky configs:plus doing things like turning on/off pydantic-validation in your configs becomes really painful.
We should provide a simply utility that allows people to set default options for builds. E.g.
Now it is trivial for people to turn this validation on/off. Obviously there are many nice use-cases here.
Feedback
Am I missing something here? Are there opportunities that I am missing? Obstacles I am not foreseeing?
Naming Things
First of all, I am going to change
hydra_meta
tozen_meta
. It makes sense for all zen-specific features to have thezen_
prefix and for only hydra-specific stuff to have thehydra_
prefix. Fortunately this hasn't been released yet, so it is no biggie. Along those lines...hydra_partial
should bezen_partial
... but I am not sure if I am prepared to make that change.What do you think about the name
zen_wrappers
? A con is that one might ask what is it wrapping, and where is it wrapping it. Butzen_wraps_target_prior_to_instantiation
isn't exactly succinct. I think we can be okay with people just copy-pasting from the docs, and power-users doing the work to understand what it means. That being said, totally up for suggestions here.Config Wrangling
Any suggestions about the aforementioned
set_defaults
? Implementation-wise? Design-wise?Entry-points
Note: I got some feedback on this and don't think we need entry-points
I am totally new to these, so I don't even know what to ask for feedback on. Where should they live?
hydra_zen.third_party.wrappers
? I wish we could be more descriptive..