Error on Parameter default kwarg usage for inputs (#1197)

**Pull Request Checklist**
- [x] Fixes #812
- [x] Tests added
- [x] Documentation/examples added
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
Currently, using the `default` kwarg for a `Parameter` in an input
Annotation or Hera's Pydantic `Input` class logs a warning message,
added in #1102. This PR makes
the code path raise an error, which can be supressed using the
`suppress_parameter_default_error` experimental feature flag for 1 minor
version until it is removed.

---------

Signed-off-by: Elliot Gunton <[email protected]>

docs/examples/workflows/experimental/new_dag_decorator_params.md

@@ -45,7 +45,7 @@
 
 
     class ConcatInput(Input):
-        word_a: Annotated[str, Parameter(name="word_a", default="")]
+        word_a: Annotated[str, Parameter(name="word_a")] = ""
         word_b: str
         concat_config: ConcatConfig = ConcatConfig(reverse=False)
 

docs/examples/workflows/experimental/new_steps_decorator_with_parallel_steps.md

@@ -30,7 +30,7 @@
 
 
     class ConcatInput(Input):
-        word_a: Annotated[str, Parameter(name="word_a", default="")]
+        word_a: Annotated[str, Parameter(name="word_a")] = ""
         word_b: str
 
 

docs/user-guides/script-annotations.md

@@ -45,7 +45,8 @@ def echo_all(
 ```
 
 The fields allowed in the `Parameter` annotations are: `name`, `enum`, and `description`, `name` will be set to the
-variable name if not provided (when exporting to YAML or viewing in the Argo UI, the `name` variable will be used).
+variable name if not provided (when exporting to YAML or viewing in the Argo UI, the `name` variable will be used). A
+`default` must be set using standard Python syntax, i.e. `x: int = 42`.
 
 ## Artifacts
 

examples/workflows/experimental/new_dag_decorator_params.py

@@ -35,7 +35,7 @@ class ConcatConfig(BaseModel):
 
 
 class ConcatInput(Input):
-    word_a: Annotated[str, Parameter(name="word_a", default="")]
+    word_a: Annotated[str, Parameter(name="word_a")] = ""
     word_b: str
     concat_config: ConcatConfig = ConcatConfig(reverse=False)
 

examples/workflows/experimental/new_steps_decorator_with_parallel_steps.py

@@ -20,7 +20,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
 
 

src/hera/shared/_global_config.py

@@ -202,6 +202,7 @@ 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

src/hera/workflows/io/_io_mixins.py

@@ -7,9 +7,10 @@
 else:
     from typing_extensions import Self
 
-from pydantic.fields import FieldInfo
 
-from hera.shared._pydantic import _PYDANTIC_VERSION, get_field_annotations, get_fields
+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 get_workflow_annotation
 from hera.shared.serialization import MISSING, serialize
 from hera.workflows._context import _context
@@ -87,8 +88,14 @@ def _get_parameters(cls, object_override: Optional[Self] = None) -> List[Paramet
                 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"
+                        "and will be removed in a future minor version, 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

src/hera/workflows/script.py

@@ -43,6 +43,7 @@
 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
@@ -511,15 +512,17 @@ class will be used as inputs, rather than the class itself.
                 artifacts.append(new_object)
             elif isinstance(new_object, Parameter):
                 if new_object.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"
+                        "and will be removed in a future minor version, use a Python default value instead. "
                     )
-                    # TODO: raise error if override flag not enabled in 5.17:
-                    # if not global_config.experimental_features["..."]:
-                    #     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 new_object.default is not None:

tests/test_decorators.py

@@ -229,8 +229,9 @@ def test_dag_is_runnable():
     )
 
 
-def test_steps_with_parallel_steps_is_runnable():
+def test_steps_with_parallel_steps_is_runnable(global_config_fixture):
     """The steps function, even with a parallel context, should be runnable as Python code."""
+    global_config_fixture.experimental_features["suppress_parameter_default_error"] = True
     from tests.workflow_decorators.steps import WorkerInput, WorkerOutput, worker
 
     assert worker(WorkerInput(value_a="hello", value_b="world")) == WorkerOutput(

tests/test_script_annotations.py

@@ -68,6 +68,8 @@ 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."""
 
     # GIVEN
+    global_config_fixture.experimental_features["suppress_parameter_default_error"] = True
+
     @script()
     def echo_int(an_int: Annotated[int, Parameter(default=1)] = 2):
         print(an_int)
@@ -83,13 +85,14 @@ def echo_int(an_int: Annotated[int, Parameter(default=1)] = 2):
     assert "default cannot be set via both the function parameter default and the Parameter's default" in str(e.value)
 
 
-@pytest.mark.skip(reason="Code change required in next Hera version")
-def test_parameter_default_throws_a_value_error(global_config_fixture):
-    """Test asserting that it is not possible to define default in the annotation."""
+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."""
 
     # GIVEN
+    global_config_fixture.experimental_features["suppress_parameter_default_error"] = False
+
     @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
@@ -100,7 +103,11 @@ def echo_int(an_int: Annotated[int, Parameter(default=1)] = 2):
 
         w.to_dict()
 
-    assert "default cannot be set via the Parameter's default, use a Python default value instead" in str(e.value)
+    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)
 
 
 @pytest.mark.parametrize(