Adds FunctionInputOutputTypeChecker #757
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a lifecycle adapter that one can add to strictly enforce the types of things at runtime. It will check the input to and the output from a function and check that it matches the expected type annotation. It modifies/fixes the pandas split-apply-combine example to conform to the expected types; it actually found mismatches between what was typed and what was passed in. I had to modify graph_functions.py so that the error message from the pre-node-execute hook would show up in the right place and the post-node-execute hook would not run. Note: this does not do anything smart with Pandera or Annotated types just yet, but it could in future. Test generated with the help of co-pilot. A little ugly, but they do the job.
They changed what was output, so test needed to be fixed.
skrawcz
force-pushed
the
add_strict_adapter
branch
from
c9b78f4 to
0915e8d
Compare
| result=result, | ||
| task_id=task_id, | ||
| ) | ||
| if not pre_node_execute_errored and adapter.does_hook( |
There was a problem hiding this comment.
Hmm we should really have a context manager that does this... But this works for now.
| @@ -206,6 +208,7 @@ def pre_node_execute( | |||
| node_return_type=node_.type, | |||
| task_id=task_id, | |||
| run_id=run_id, | |||
| node_input_types={k: v[0] for k, v in node_.input_types.items()}, | |||
There was a problem hiding this comment.
Add TODO for integrating the HamiltonNode class
hamilton/lifecycle/default.py
Outdated
| import time | ||
| from typing import Any, Callable, Dict, List, Optional, Union | ||
|
|
||
| if sys.version_info >= (3, 9): | ||
| from typing import Literal |
There was a problem hiding this comment.
Think literal is 3.8+?
To centralize this and make it clear we have two ways of checking types. I don't want to merge them because the input type checker is less strict. It doesn't recursively go check the values of inputs. So there's a TODO: to figure out what to do, and how to unify. In the meantime they're in the same file.
elijahbenizzy
approved these changes
Mar 12, 2024
Also adds more to example
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a lifecycle adapter that one can add to strictly enforce the types of things at runtime. It will check the input to and the output from a function and check that it matches the expected type annotation.
It modifies/fixes the pandas split-apply-combine example to conform to the expected types; it actually found mismatches between what was typed and what was passed in.
I had to modify graph_functions.py so that the error message from the pre-node-execute hook would show up in the right place and the post-node-execute hook would not run.
Note: this does not do anything smart with Pandera or Annotated types just yet, but it could in future.
Test generated with the help of co-pilot. A little ugly, but they do the job.
Changes
How I tested this
Notes
Checklist