diff --git a/THIRD-PARTY-LICENSES.txt b/THIRD-PARTY-LICENSES.txt index a89a1057..f749ff3b 100644 --- a/THIRD-PARTY-LICENSES.txt +++ b/THIRD-PARTY-LICENSES.txt @@ -24,7 +24,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ------ -** pydantic; version 2.13.4 -- https://pypi.org/project/pydantic/ +** pydantic; version 2.13.5 -- https://pypi.org/project/pydantic/ The MIT License (MIT) Copyright (c) 2017 to present Pydantic Services Inc. and individual contributors. @@ -48,7 +48,7 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. ------ -** pydantic_core; version 2.46.4 -- https://pypi.org/project/pydantic_core/ +** pydantic_core; version 2.46.5 -- https://pypi.org/project/pydantic_core/ The MIT License (MIT) Copyright (c) 2022 Samuel Colvin diff --git a/src/openjd/model/_step_param_space_iter.py b/src/openjd/model/_step_param_space_iter.py index d3aec120..dbcd41d5 100644 --- a/src/openjd/model/_step_param_space_iter.py +++ b/src/openjd/model/_step_param_space_iter.py @@ -338,7 +338,14 @@ def _create_expr_tree( name=name, type=ParameterValueType(parameter.type), range=parameter.range, - range_set=set(parameter.range), + # Non-CHUNK containment compares against ParameterValue.value, + # which is the rendered form of a range element (see + # RangeListIdentifierNode.__getitem__), so the set has to hold + # rendered elements too. A set of the raw elements only matched + # when a range happened to be written as strings — an INT range + # written as `[1, 2, 3]` reported every one of its own values as + # not contained. + range_set={str(v) for v in parameter.range}, ) else: return RangeExpressionIdentifierNode( diff --git a/src/openjd/model/v2023_09/_model.py b/src/openjd/model/v2023_09/_model.py index 763fb053..56caa30c 100644 --- a/src/openjd/model/v2023_09/_model.py +++ b/src/openjd/model/v2023_09/_model.py @@ -1208,7 +1208,10 @@ def __new__(cls, value: str, *, context: ModelParsingContextInterface = ModelPar list[Union[Decimal, TaskParameterStringValue]], Field(min_length=1, max_length=1024) ] StringRangeList = Annotated[list[TaskParameterStringValue], Field(min_length=1, max_length=1024)] -TaskParameterStringValueAsJob = Annotated[str, StringConstraints(min_length=0, max_length=1024)] +_MAX_TASK_PARAM_VALUE_LEN = 1024 +TaskParameterStringValueAsJob = Annotated[ + str, StringConstraints(min_length=0, max_length=_MAX_TASK_PARAM_VALUE_LEN) +] TaskRangeList = list[Union[TaskParameterStringValueAsJob, int, float, Decimal]] @@ -1242,6 +1245,76 @@ def _validate_target_runtime_seconds(cls, value: Any, info: ValidationInfo) -> A return validate_int_fmtstring_field(value, ge=0, context=context) +def _normalized_range_element(elem: str, to_int: bool) -> Any: + """The number an ````/```` range element denotes -- an + ``int`` for ````, and for ```` the plain-notation text + of the value, since ``str(Decimal)`` cannot render every magnitude without an + exponent. Returns the element unchanged when it does not denote a number, or + when its plain-notation form would not fit this field's character cap.""" + try: + if to_int: + return int(elem) + value = Decimal(elem) + if not value.is_finite(): + # Decimal accepts 'nan'/'inf', which are not s; leave + # them as text rather than changing how they render. + return elem + # Bound the expansion from the exponent before performing it. Decimal + # construction from a string is unbounded -- the context's Emax/Emin + # constrain arithmetic results, not construction, so Decimal('1e999999999') + # is finite and the guard above does not stop it -- and the plain-notation + # length is linear in the exponent, so 11 characters of template text + # would otherwise materialize ~10**9 characters. + # + # The bound is the character cap this field already enforces, because an + # expansion past it cannot be carried here as text at all: the value is + # stored in a TaskRangeList element, whose str member is + # TaskParameterStringValueAsJob, and pydantic's union falls through to + # the numeric members instead, rendering the element as an integer, or as + # inf, or as 0.0. Such an element keeps its source text -- what this + # function already does for anything it cannot normalize, and how it + # rendered before normalization was introduced. + exponent = cast(int, value.as_tuple().exponent) # finite: never 'n'/'N'/'F' + # Length of the '[-].' text returned below. Stripping + # redundant zeros can only shorten it, so this is an upper bound. + expansion_len = ( + (1 if value.is_signed() else 0) + + max(value.adjusted() + 1, 1) # integer digits, at least one + + 1 # the decimal point + + max(-exponent, 1) # fraction digits, at least one + ) + if expansion_len > _MAX_TASK_PARAM_VALUE_LEN: + return elem + # format(value, 'f') is exact and plain at every magnitude: with no + # precision in the format spec it neither rounds to getcontext().prec + # nor switches to exponent notation, so an embedding app that sets a + # different context cannot change what this library renders. Neither + # holds of the obvious alternatives -- normalize() rounds to the + # ambient precision, str() switches to exponent notation below 1e-6, + # and quantize() raises InvalidOperation once the result needs more + # digits than the ambient precision allows. + text = format(value, "f") + negative = text.startswith("-") + integer, _, fraction = text.lstrip("-").partition(".") + # A denotes a number, so redundant zeros in its source + # text are not part of the value: '02.50' is 2.5. One fractional digit + # is always kept, matching openjd-rs, which renders the float 1.0 as + # `1.0` and never as `1`. + integer = integer.lstrip("0") or "0" + fraction = fraction.rstrip("0") or "0" + if negative and integer == "0" and fraction == "0": + # The number denoted by '-0.0' is zero, which has no sign; openjd-rs + # renders it `0.0`. + negative = False + return f"{'-' if negative else ''}{integer}.{fraction}" + except (ValueError, ArithmeticError): + # Not a number at all — e.g. a format string that resolved to + # non-numeric text. A literal range is already checked against its + # element type at template parse time, so leave such a value to the + # existing behaviour rather than adding a rejection path here. + return elem + + # Target model for task parameters when instantiating a job. class RangeListTaskParameterDefinition(OpenJDModel_v2023_09): # element type of items in the range @@ -1251,6 +1324,38 @@ class RangeListTaskParameterDefinition(OpenJDModel_v2023_09): # has a value when type is CHUNK[INT], which is only possible from the TASK_CHUNKING extension chunks: Optional[TaskChunksDefinition] = None + @field_validator("range", mode="before") + @classmethod + def _normalize_numeric_range_elements(cls, value: Any, info: ValidationInfo) -> Any: + # §3.4.1.1/§3.4.1.2: an element is ` | + # ` and a element is ` | + # `, where an / is "a string whose + # value is the string representation of" a number (§2.3, §2.4). Such an + # element therefore denotes that number, not its source text, so '02' + # is the task value 2 and '02.50' is 2.5. Python was keeping the source + # text, which reaches a task command line as `--frame 02`. + # + # Applied on the instantiation target so every inbound range path is + # covered: a literal list, a range-expression expansion, and an RFC 0006 + # typed whole-field resolution all funnel through this model. + # + # Numeric elements are left exactly as parsed. A FLOAT range of [1.0] + # renders 1.0, which openjd-rs also does and the conformance suite pins + # (base/jobs/3.4--float-parameter). STRING and PATH ranges are text by + # definition and are never touched. + param_type = info.data.get("type") + if not isinstance(value, list) or param_type not in ( + TaskParameterType.INT, + TaskParameterType.CHUNK_INT, + TaskParameterType.FLOAT, + ): + return value + to_int = param_type != TaskParameterType.FLOAT + return [ + _normalized_range_element(elem, to_int) if isinstance(elem, str) else elem + for elem in value + ] + @field_validator("range") @classmethod def _validate_range_len(cls, value: Any) -> Any: diff --git a/test/openjd/model_v0/test_step_param_space_iter.py b/test/openjd/model_v0/test_step_param_space_iter.py index e059dd00..d11e277d 100644 --- a/test/openjd/model_v0/test_step_param_space_iter.py +++ b/test/openjd/model_v0/test_step_param_space_iter.py @@ -567,3 +567,102 @@ def test_nested_expr_iteration(self) -> None: "Param3": ParameterValue(type=ParameterValueType.STRING, value="11"), "Param4": ParameterValue(type=ParameterValueType.INT, value="20"), } not in it + + +class TestRangeListElementValues: + """The values a task command line actually receives, end to end from a + template through `create_job`. + + §3.4.1.1/§3.4.1.2 allow a range element to be written in the `` / + `` form, which §2.3/§2.4 define as "a string whose value is the + string representation of" a number. Such an element denotes the number, so + `['1', '02', '003']` on an INT parameter is the task values 1, 2 and 3 — not + the source text, which would reach a renderer as `--frame 02`. + """ + + @staticmethod + def _task_values(param_type: str, range_list: list) -> list[str]: + job_template = parse_model( + model=JobTemplate_2023_09, + obj={ + "specificationVersion": "jobtemplate-2023-09", + "name": "Job", + "steps": [ + { + "name": "step", + "parameterSpace": { + "taskParameterDefinitions": [ + {"name": "P", "type": param_type, "range": range_list} + ] + }, + "script": {"actions": {"onRun": {"command": "do thing"}}}, + } + ], + }, + ) + job = create_job(job_template=job_template, job_parameter_values=dict()) + return [ + params["P"].value + for params in StepParameterSpaceIterator(space=job.steps[0].parameterSpace) + ] + + def test_int_range_intstring_elements(self) -> None: + # WHEN an INT range is written with elements + # THEN each task gets the integer the element denotes + assert self._task_values("INT", ["1", "02", "003"]) == ["1", "2", "3"] + + def test_float_range_floatstring_elements(self) -> None: + # WHEN a FLOAT range is written with elements + # THEN each task gets the number the element denotes + assert self._task_values("FLOAT", ["1.5", "02.50"]) == ["1.5", "2.5"] + + def test_float_range_numeric_literals_keep_their_scale(self) -> None: + # A literal is not a string representation, so it keeps the scale + # it was written with. openjd-rs renders an integral float as `1.0` too, + # and the conformance suite pins this + # (2023-09/base/jobs/3.4--float-parameter). + assert self._task_values("FLOAT", [0.5, 1.0, 1.5]) == ["0.5", "1.0", "1.5"] + + @pytest.mark.parametrize( + "param_type,range_list", + ( + pytest.param("STRING", ["02", "1.50"], id="STRING"), + pytest.param("PATH", ["/frames/02", "/frames/003"], id="PATH"), + ), + ) + def test_string_and_path_ranges_are_text(self, param_type: str, range_list: list) -> None: + # §3.4.1.3/§3.4.1.4 give STRING and PATH no numeric element form, so + # nothing about their elements is normalizable. + assert self._task_values(param_type, range_list) == range_list + + @pytest.mark.parametrize( + "range_list", + ( + pytest.param([10, 11, 12], id="written as integers"), + pytest.param(["10", "11", "12"], id="written as intstrings"), + ), + ) + def test_containment_matches_the_rendered_values(self, range_list: list) -> None: + # Containment compares a ParameterValue against the parameter space, and + # ParameterValue.value is the *rendered* form of a range element. The + # containment set was built from the raw elements, so an INT range + # written as `[10, 11, 12]` reported its own values as not contained. + + # GIVEN a parameter space over an int range + space = StepParameterSpace_2023_09( + taskParameterDefinitions={ + "Param1": RangeListTaskParameterDefinition_2023_09( + type=ParameterValueType.INT, range=range_list + ), + }, + ) + + # WHEN it is iterated + it = StepParameterSpaceIterator(space=space) + + # THEN every value it yields is contained in it, however the range was written + values = list(it) + assert [v["Param1"].value for v in values] == ["10", "11", "12"] + for value in values: + assert value in it + assert {"Param1": ParameterValue(type=ParameterValueType.INT, value="13")} not in it diff --git a/test/openjd/model_v0/v2023_09/test_parameter_space.py b/test/openjd/model_v0/v2023_09/test_parameter_space.py index 23f0af9e..25ae76aa 100644 --- a/test/openjd/model_v0/v2023_09/test_parameter_space.py +++ b/test/openjd/model_v0/v2023_09/test_parameter_space.py @@ -1,5 +1,6 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +from decimal import localcontext from typing import Any import pytest @@ -439,6 +440,180 @@ def test_list_form_range_is_still_capped(self, model: Any, obj: dict[str, Any]) assert len(excinfo.value.errors()) > 0 +class TestRangeListElementNormalization: + """§3.4.1.1/§3.4.1.2: an `` element is ` | ` + and a `` element is ` | `. An + ``/`` is "a string whose value is the string + representation of" a number (§2.3, §2.4), so it denotes that number and not + its source text — a range of `['1', '02', '003']` is the task values 1, 2 and + 3. These values reach a task command line, so keeping the text renders + `--frame 02`. + """ + + @pytest.mark.parametrize( + "param_type", + (pytest.param("INT", id="INT"), pytest.param("CHUNK[INT]", id="CHUNK[INT]")), + ) + def test_intstring_elements_carry_their_value(self, param_type: str) -> None: + # GIVEN an int range mixing the and forms + obj: dict[str, Any] = {"type": param_type, "range": [1, "02", "003", 4]} + if param_type == "CHUNK[INT]": + obj["chunks"] = {"defaultTaskCount": 1, "rangeConstraint": "CONTIGUOUS"} + + # WHEN the instantiation target parses it + model = _parse_model(model=RangeListTaskParameterDefinition, obj=obj) + + # THEN every element is the integer it denotes, and renders without the + # leading zeros of its source text + assert model.range == [1, 2, 3, 4] + assert [str(v) for v in model.range] == ["1", "2", "3", "4"] + + @pytest.mark.parametrize( + "element,expected", + ( + pytest.param("1.5", "1.5", id="no normalization needed"), + pytest.param("02.50", "2.5", id="leading and trailing zeros"), + pytest.param("3.500", "3.5", id="trailing zeros"), + pytest.param("-02.50", "-2.5", id="negative"), + # An integral float keeps one fractional digit. openjd-rs renders + # the '1.0' as `1.0` and '007' as `7.0`, so dropping + # the fraction would make the same number render two ways depending + # on whether it was written as a or a . + pytest.param("1.0", "1.0", id="integral float"), + pytest.param("0.0", "0.0", id="zero"), + pytest.param("-0.0", "0.0", id="negative zero has no sign"), + pytest.param("007", "7.0", id="leading zeros on a whole number"), + pytest.param("100", "100.0", id="whole number"), + # Exponent notation must never reach a task command line, at any + # magnitude and in either direction. + pytest.param("1E+2", "100.0", id="exponent notation in the source"), + pytest.param( + "0.0000001", + "0.0000001", + id="below 1e-6, where Decimal.__str__ would give 1E-7", + ), + pytest.param( + "1E+30", + "1" + "0" * 30 + ".0", + id="more digits than the default decimal precision", + ), + pytest.param( + "1.2345678901234567890123456789012345678901", + "1.2345678901234567890123456789012345678901", + id="more significant digits than the default decimal precision", + ), + ), + ) + def test_floatstring_elements_carry_their_value(self, element: str, expected: str) -> None: + # WHEN the instantiation target parses a float range holding a + model = _parse_model( + model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]} + ) + + # THEN the element renders as the number it denotes + assert [str(v) for v in model.range] == [expected] + + @pytest.mark.parametrize( + "element", + ( + # Decimal's context bounds arithmetic results, not construction from + # a string, so these are finite: 11 characters of template text + # whose plain-notation expansion is ~10**9 characters. + pytest.param("1e999999999", id="huge positive exponent"), + pytest.param("1e-999999999", id="huge negative exponent"), + # One character past the cap in each direction: both expand to 1025. + pytest.param("1E+1022", id="one past the cap, positive exponent"), + pytest.param("1E-1023", id="one past the cap, negative exponent"), + ), + ) + def test_element_too_long_to_expand_keeps_its_source_text(self, element: str) -> None: + # WHEN the instantiation target parses a whose plain-notation + # form would not fit TaskParameterStringValueAsJob's 1024-character cap + model = _parse_model( + model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]} + ) + + # THEN it keeps its source text rather than being expanded. Expanding it + # would allocate the whole expansion, and the result could not be carried + # as text here anyway: it fails the str member of TaskRangeList's union, + # and pydantic then falls through to the numeric members, rendering the + # element as an integer, or as inf, or as 0.0. + assert [str(v) for v in model.range] == [element] + + @pytest.mark.parametrize( + "element,expected", + ( + pytest.param("1E+1021", "1" + "0" * 1021 + ".0", id="positive exponent"), + pytest.param("1E-1022", "0." + "0" * 1021 + "1", id="negative exponent"), + ), + ) + def test_element_at_the_cap_is_still_normalized(self, element: str, expected: str) -> None: + # GIVEN a whose plain-notation form is exactly 1024 + # characters — at the cap, not past it + assert len(expected) == 1024 + + # WHEN the instantiation target parses it + model = _parse_model( + model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]} + ) + + # THEN the length bound has not cost it its normalization + assert [str(v) for v in model.range] == [expected] + + def test_floatstring_normalization_ignores_the_decimal_context(self) -> None: + # GIVEN an embedding application that has narrowed the process-wide + # decimal context, and a with more significant digits + # than that precision allows + element = "1.2345678901234567890123456789012345678901" + + # WHEN the instantiation target parses it under that context + with localcontext() as ctx: + ctx.prec = 5 + model = _parse_model( + model=RangeListTaskParameterDefinition, obj={"type": "FLOAT", "range": [element]} + ) + + # THEN the value is unrounded: what this library renders is a property of + # the template, not of the host application's decimal context + assert [str(v) for v in model.range] == [element] + + @pytest.mark.parametrize( + "param_type,range_list", + ( + # A literal keeps the scale it was written with. openjd-rs + # renders an integral float as `1.0` too, and the conformance suite + # pins it (2023-09/base/jobs/3.4--float-parameter). + pytest.param("FLOAT", [0.5, 1.0, 1.5], id="FLOAT numeric literals"), + pytest.param("INT", [1, 2, 3], id="INT numeric literals"), + # STRING and PATH ranges are text by definition — §3.4.1.3/§3.4.1.4 + # give no numeric form, so nothing about them is normalizable. + pytest.param("STRING", ["02", "003", "1.50"], id="STRING"), + pytest.param("PATH", ["/frames/02", "/frames/003"], id="PATH"), + ), + ) + def test_elements_that_must_not_change(self, param_type: str, range_list: list) -> None: + # WHEN the instantiation target parses a range that normalization must not touch + model = _parse_model( + model=RangeListTaskParameterDefinition, + obj={"type": param_type, "range": range_list}, + ) + + # THEN every element renders exactly as it was written + assert [str(v) for v in model.range] == [str(v) for v in range_list] + + def test_unparseable_element_is_left_alone(self) -> None: + # GIVEN a range element that does not denote a number — reachable when a + # format string resolves to non-numeric text, since the template-layer + # element check only sees literals. + # WHEN the instantiation target parses it + model = _parse_model( + model=RangeListTaskParameterDefinition, obj={"type": "INT", "range": ["notanumber"]} + ) + + # THEN it is carried through unchanged rather than rejected here + assert model.range == ["notanumber"] + + class TestStepParameterSpaceDefinition: @pytest.mark.parametrize( "data",