Skip to content

Expose default values in HamiltonNode #710

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

Closed
elijahbenizzy opened this issue Feb 22, 2024 · 1 comment · Fixed by #1035
Closed

Expose default values in HamiltonNode #710

elijahbenizzy opened this issue Feb 22, 2024 · 1 comment · Fixed by #1035
Labels
enhancement New feature or request

Comments

@elijahbenizzy
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
We have optional_dependencies but its just a set. We don't expose it.

Describe the solution you'd like
We should have a default_values field or something like that. Dict[str, Any].

Describe alternatives you've considered
People can inspect the function -- see https://github.com/DAGWorks-Inc/hamilton/blob/main/hamilton/plugins/h_experiments/hook.py for instance.

Additional context
Feature request from OS. This is tricky as we don't capture it, but should be possible with a bit of rewiring.

@zilto
Copy link
Collaborator

zilto commented Mar 3, 2024

relates to #495

@skrawcz skrawcz added the enhancement New feature or request label Jul 18, 2024
skrawcz added a commit that referenced this issue Jul 18, 2024
This fixes #710.

This hasn't been extensively tested, but this
seems to work and pass things through as
expected. There is a JSON serialization
caveat, however we expect default
values to be primitives so that should
be fine. If not the user can augment the
dictionary returned ...
skrawcz added a commit that referenced this issue Aug 28, 2024
This fixes #710.

This hasn't been extensively tested, but this
seems to work and pass things through as
expected. There is a JSON serialization
caveat, however we expect default
values to be primitives so that should
be fine. If not the user can augment the
dictionary returned ...
skrawcz added a commit that referenced this issue Aug 28, 2024
* Implements #710 optional value exposure

This fixes #710.

This hasn't been extensively tested, but this
seems to work and pass things through as
expected. There is a JSON serialization
caveat, however we expect default
values to be primitives so that should
be fine. We default to not returning optional
values in this case, and users must request it
themselves.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants