Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixed optional typing on non-serializable types #2939

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fixed optional typing on non-serializable types #2939

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3635f11
Fixed optional typing on non-serializable types
AlexejPenner Aug 21, 2024
ba67f40
Applied reviews and simplified the fix
AlexejPenner Aug 27, 2024
04e627a
Adjusted test cases
AlexejPenner Aug 27, 2024
15d5475
Merge branch 'develop' into bugfix/GROW-96-fix-optional-typing
AlexejPenner Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions src/zenml/steps/entrypoint_function_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
Union,
)

from pydantic import ConfigDict, ValidationError, create_model
from pydantic import ValidationError, create_model, ConfigDict

from zenml.constants import ENFORCE_TYPE_ANNOTATIONS
from zenml.exceptions import StepInterfaceError
Expand Down Expand Up @@ -197,17 +197,23 @@ def _validate_input_value(
parameter: The function parameter for which the value was provided.
value: The input value.
"""
config_dict = ConfigDict(arbitrary_types_allowed=False)

# Create a pydantic model with just a single required field with the
# type annotation of the parameter to verify the input type including
# pydantics type coercion
validation_model_class = create_model(
"input_validation_model",
__config__=config_dict,
value=(parameter.annotation, ...),
)
validation_model_class(value=value)
annotation = parameter.annotation

# Use Pydantic for all types to take advantage of its coercion abilities
try:
config_dict = ConfigDict(arbitrary_types_allowed=True)
validation_model_class = create_model(
"input_validation_model",
__config__=type("Config", (), config_dict),
value=(annotation, ...),
)
validation_model_class(value=value)
except Exception:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be

try:
  ...
except ValidationError:
  raise
except Exception:
  ...

instead? In case pydantic fails with a regular validation error, we actually want to raise that because it means the types don't match right?

# If Pydantic can't handle it, fall back to isinstance
if not isinstance(value, annotation):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this might fail with the raw annotation:

isinstance(int, Union[int, str])

# TypeError: Subscripted generics cannot be used with class and instance checks

raise TypeError(
f"Expected {annotation}, but got {type(value)}"
)


def validate_entrypoint_function(
Expand Down
Loading