diff --git a/src/hera/shared/_global_config.py b/src/hera/shared/_global_config.py index a5c42353c..a93b3731b 100644 --- a/src/hera/shared/_global_config.py +++ b/src/hera/shared/_global_config.py @@ -202,7 +202,6 @@ def _set_defaults(cls, values): _SCRIPT_ANNOTATIONS_FLAG = "script_annotations" _SCRIPT_PYDANTIC_IO_FLAG = "script_pydantic_io" _DECORATOR_SYNTAX_FLAG = "decorator_syntax" -_SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG = "suppress_parameter_default_error" # A dictionary where each key is a flag that has a list of flags which supersede it, hence # the given flag key can also be switched on by any of the flags in the list. Using simple flat lists diff --git a/src/hera/workflows/io/_io_mixins.py b/src/hera/workflows/io/_io_mixins.py index 722d4d8bc..98f7804ee 100644 --- a/src/hera/workflows/io/_io_mixins.py +++ b/src/hera/workflows/io/_io_mixins.py @@ -1,5 +1,4 @@ import sys -import warnings from typing import TYPE_CHECKING, Iterator, List, Optional, Tuple, Type, Union if sys.version_info >= (3, 11): @@ -8,8 +7,6 @@ from typing_extensions import Self -from hera.shared import global_config -from hera.shared._global_config import _SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG from hera.shared._pydantic import _PYDANTIC_VERSION, FieldInfo, get_field_annotations, get_fields from hera.shared._type_util import construct_io_from_annotation, get_workflow_annotation from hera.shared.serialization import MISSING, serialize @@ -80,16 +77,9 @@ def _get_parameters(cls, object_override: Optional[Self] = None) -> List[Paramet for field, field_info, param in _construct_io_from_fields(cls): if isinstance(param, Parameter): if param.default is not None: - warnings.warn( - "Using the default field for Parameters in Annotations is deprecated since v5.16" - "and will be removed in a future minor version, use a Python default value instead. " + raise ValueError( + "default cannot be set via the Parameter's default, use a Python default value instead." ) - if not global_config.experimental_features[_SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG]: - raise ValueError( - "default cannot be set via the Parameter's default, use a Python default value instead. " - "You can suppress this error by setting " - f'global_config.experimental_features["{_SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG}"] = True' - ) if object_override: param.default = serialize(getattr(object_override, field)) elif field_info.default is not None and field_info.default != PydanticUndefined: # type: ignore diff --git a/src/hera/workflows/script.py b/src/hera/workflows/script.py index 2065c438c..ad066380a 100644 --- a/src/hera/workflows/script.py +++ b/src/hera/workflows/script.py @@ -9,7 +9,6 @@ import inspect import sys import textwrap -import warnings from abc import abstractmethod from functools import wraps from typing import ( @@ -43,7 +42,6 @@ from hera.shared._global_config import ( _SCRIPT_ANNOTATIONS_FLAG, _SCRIPT_PYDANTIC_IO_FLAG, - _SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG, _flag_enabled, ) from hera.shared._pydantic import _PYDANTIC_VERSION, root_validator, validator @@ -509,23 +507,10 @@ class will be used as inputs, rather than the class itself. artifacts.append(io) elif isinstance(io, Parameter): if io.default is not None: - # TODO: in 5.18 remove the flag check and `warn`, and raise the ValueError directly (minus "flag" text) - warnings.warn( - "Using the default field for Parameters in Annotations is deprecated since v5.16" - "and will be removed in a future minor version, use a Python default value instead. " + raise ValueError( + "default cannot be set via the Parameter's default, use a Python default value instead." ) - if not global_config.experimental_features[_SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG]: - raise ValueError( - "default cannot be set via the Parameter's default, use a Python default value instead" - "You can suppress this error by setting " - f'global_config.experimental_features["{_SUPPRESS_PARAMETER_DEFAULT_ERROR_FLAG}"] = True' - ) if func_param.default != inspect.Parameter.empty: - # TODO: remove this check in 5.18: - if io.default is not None: - raise ValueError( - "default cannot be set via both the function parameter default and the Parameter's default" - ) io.default = serialize(func_param.default) if origin_type_issupertype(func_param.annotation, NoneType) and io.default != "null": diff --git a/tests/test_script_annotations.py b/tests/test_script_annotations.py index 93fe49e36..028328af2 100644 --- a/tests/test_script_annotations.py +++ b/tests/test_script_annotations.py @@ -7,6 +7,7 @@ from hera.shared._pydantic import _PYDANTIC_VERSION from hera.workflows import Workflow, script +from hera.workflows.io import Input from hera.workflows.parameter import Parameter from hera.workflows.steps import Steps @@ -64,14 +65,12 @@ def test_script_annotations_artifact_regression(module_name, global_config_fixtu _compare_workflows(workflow_old, output_old, output_new) -def test_double_default_throws_a_value_error(global_config_fixture): - """Test asserting that it is not possible to define default in the annotation and normal Python.""" +def test_parameter_default_throws_a_value_error(global_config_fixture): + """Test asserting that it is not possible to define default in the annotation.""" # GIVEN - global_config_fixture.experimental_features["suppress_parameter_default_error"] = True - @script() - def echo_int(an_int: Annotated[int, Parameter(default=1)] = 2): + def echo_int(an_int: Annotated[int, Parameter(default=1)]): print(an_int) global_config_fixture.experimental_features["script_annotations"] = True @@ -82,18 +81,21 @@ def echo_int(an_int: Annotated[int, Parameter(default=1)] = 2): w.to_dict() - assert "default cannot be set via both the function parameter default and the Parameter's default" in str(e.value) + assert ("default cannot be set via the Parameter's default, use a Python default value instead") in str(e.value) -def test_parameter_default_without_suppression_throws_a_value_error(global_config_fixture): - """Test asserting that it is not possible to define default in the annotation and normal Python.""" +def test_pydantic_input_with_default_throws_a_value_error(global_config_fixture): + """Test asserting that it is not possible to define default in the annotation in a Hera Input class.""" # GIVEN - global_config_fixture.experimental_features["suppress_parameter_default_error"] = False + global_config_fixture.experimental_features["script_pydantic_io"] = True + + class ExampleInput(Input): + an_int: Annotated[int, Parameter(default=1)] @script() - def echo_int(an_int: Annotated[int, Parameter(default=1)]): - print(an_int) + def echo_int(pydantic_input: ExampleInput): + print(pydantic_input.an_int) global_config_fixture.experimental_features["script_annotations"] = True with pytest.raises(ValueError) as e: @@ -103,11 +105,7 @@ def echo_int(an_int: Annotated[int, Parameter(default=1)]): w.to_dict() - assert ( - "default cannot be set via the Parameter's default, use a Python default value instead" - "You can suppress this error by setting " - 'global_config.experimental_features["suppress_parameter_default_error"] = True' - ) in str(e.value) + assert ("default cannot be set via the Parameter's default, use a Python default value instead") in str(e.value) @pytest.mark.parametrize( diff --git a/tests/workflow_decorators/steps.py b/tests/workflow_decorators/steps.py index 4e507c128..83f49124d 100644 --- a/tests/workflow_decorators/steps.py +++ b/tests/workflow_decorators/steps.py @@ -22,7 +22,7 @@ def setup() -> SetupOutput: class ConcatInput(Input): - word_a: Annotated[str, Parameter(name="word_a", default="")] + word_a: Annotated[str, Parameter(name="word_a")] = "" word_b: str