From b171d0bfec94a99d31197b22b982c3a2468568ac Mon Sep 17 00:00:00 2001 From: Builder106 Date: Sat, 30 May 2026 10:46:03 -0700 Subject: [PATCH] Fix docs: non-string annotations are converted, not rejected The docstrings in dag.py, the exception table in docs/api.md, and the usage_patterns notebook all stated that `set_annotations=True` requires string annotations and raises `NonStringAnnotationError` otherwise. This has been inaccurate since #67 ("do not require string annotations from users"): non-string annotations are converted to their string representation via `ensure_annotations_are_strings`, and `NonStringAnnotationError` is never raised anywhere in the code base (nor is it exported or covered by tests). - Update the stale docstrings (FunctionExecutionInfo, concatenate_functions, _create_combined_function_from_dag, _create_concatenated_function, create_execution_info) to describe the actual convert behavior and drop the false `NonStringAnnotationError` Raises entries. - Update the docs/api.md exception table and the usage_patterns notebook. - Add a regression test asserting that concatenate_functions(set_annotations= True) converts non-string annotations instead of raising. --- CHANGES.md | 5 +++ docs/api.md | 2 +- docs/usage_patterns.ipynb | 2 +- src/dags/dag.py | 50 ++++++++---------------- tests/test_without_string_annotations.py | 36 ++++++++++++++++- 5 files changed, 59 insertions(+), 36 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 65bd473..18bde52 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,11 @@ releases are available on [conda-forge](https://anaconda.org/conda-forge/dags). ## Unreleased +- Fix docstrings and docs that incorrectly stated `set_annotations=True` requires + string annotations and raises `NonStringAnnotationError`. Since :gh:`67`, non-string + annotations are converted to their string representation and the exception is never + raised. Added a regression test for this behavior. + ## 0.6.0 - :gh:`82` Make dags wrappers play nicely with runtime type checkers diff --git a/docs/api.md b/docs/api.md index c1ce868..ee36895 100644 --- a/docs/api.md +++ b/docs/api.md @@ -401,7 +401,7 @@ ______________________________________________________________________ | `CyclicDependencyError` | `dags.exceptions` | DAG contains a cycle | | `MissingFunctionsError` | `dags.exceptions` | Target functions not provided | | `AnnotationMismatchError` | `dags.exceptions` | Type annotations conflict between functions | -| `NonStringAnnotationError` | `dags.exceptions` | Non-string annotation with `set_annotations=True` | +| `NonStringAnnotationError` | `dags.exceptions` | Reserved; not currently raised — non-string annotations are converted to strings | | `InvalidFunctionArgumentsError` | `dags.exceptions` | Invalid function arguments (too many positional, duplicated, unexpected keyword) | | `ValidationError` | `dags.exceptions` | Base for validation errors | | `RepeatedTopLevelElementError` | `dags.tree.exceptions` | Top-level element repeated in tree path | diff --git a/docs/usage_patterns.ipynb b/docs/usage_patterns.ipynb index 89ac3ea..62ea038 100644 --- a/docs/usage_patterns.ipynb +++ b/docs/usage_patterns.ipynb @@ -962,7 +962,7 @@ { "cell_type": "markdown", "metadata": {}, - "source": "5. **Set annotations for type checking**: When `set_annotations=True`, dags propagates type annotations from the individual functions to the combined function. This is useful for introspection and documentation. All annotations must be strings (use `from __future__ import annotations` or quote them). dags will raise `AnnotationMismatchError` if the same argument gets conflicting type annotations from different functions:" + "source": "5. **Set annotations for type checking**: When `set_annotations=True`, dags propagates type annotations from the individual functions to the combined function. This is useful for introspection and documentation. Annotations need not be strings: non-string annotations (e.g. when `from __future__ import annotations` is not used) are converted to their string representation. dags will raise `AnnotationMismatchError` if the same argument gets conflicting type annotations from different functions:" }, { "cell_type": "code", diff --git a/src/dags/dag.py b/src/dags/dag.py index 41accae..43b012f 100644 --- a/src/dags/dag.py +++ b/src/dags/dag.py @@ -46,17 +46,12 @@ class FunctionExecutionInfo: coincides with the __annotations__ attribute of the function. For partialled functions, this is a dictionary with the names of the free arguments as keys and their expected types as values, as well as the return type of the - function stored under the key "return". Type annotations must be strings, - else a NonStringAnnotationError is raised. + function stored under the key "return". When `verify_annotations` is True, + non-string annotations are converted to their string representation. arguments: The names of the arguments of the function. argument_annotations: The argument annotations of the function. return_annotation: The return annotation of the function. - Raises: - ------ - NonStringAnnotationError: If `verify_annotations` is `True` and the type - annotations are not strings. - """ name: str @@ -135,11 +130,11 @@ def concatenate_functions( # noqa: PLR0913 return annotation of the concatenated function reflects the requested return type and number of targets (e.g., for two targets returned as a list, the return annotation is a list of their respective type hints). Note that this - is not a valid type annotation and should not be used for type checking. All - annotations must be strings; otherwise, a NonStringAnnotationError is - raised. To ensure string annotations, enclose them in quotes or use "from - __future__ import annotations" at the top of your file. An - AnnotationMismatchError is raised if annotations differ between functions. + is not a valid type annotation and should not be used for type checking. + Annotations need not be strings: non-string annotations (e.g. when "from + __future__ import annotations" is not used, or on Python 3.14+) are + converted to their string representation. An AnnotationMismatchError is + raised if annotations differ between functions. lexsort_key (callable or None): A function that takes a string and returns a value that can be used to sort the nodes. This is used to sort the nodes in the topological sort. If None, the nodes are sorted alphabetically. @@ -150,9 +145,6 @@ def concatenate_functions( # noqa: PLR0913 Raises: ------ - - NonStringAnnotationError: If `set_annotations` is `True` and the type - annotations are not strings. - - AnnotationMismatchError: If `set_annotations` is `True` and there are incompatible annotations in the DAG's components. @@ -254,11 +246,11 @@ def _create_combined_function_from_dag( # noqa: PLR0913 return annotation of the concatenated function reflects the requested return type and number of targets (e.g., for two targets returned as a list, the return annotation is a list of their respective type hints). Note that this - is not a valid type annotation and should not be used for type checking. All - annotations must be strings; otherwise, a NonStringAnnotationError is - raised. To ensure string annotations, enclose them in quotes or use "from - __future__ import annotations" at the top of your file. An - AnnotationMismatchError is raised if annotations differ between functions. + is not a valid type annotation and should not be used for type checking. + Annotations need not be strings: non-string annotations (e.g. when "from + __future__ import annotations" is not used, or on Python 3.14+) are + converted to their string representation. An AnnotationMismatchError is + raised if annotations differ between functions. lexsort_key (callable or None): A function that takes a string and returns a value that can be used to sort the nodes. This is used to sort the nodes in the topological sort. If None, the nodes are sorted alphabetically. @@ -269,9 +261,6 @@ def _create_combined_function_from_dag( # noqa: PLR0913 Raises: ------ - - NonStringAnnotationError: If `set_annotations` is `True` and the type - annotations are not strings. - - AnnotationMismatchError: If `set_annotations` is `True` and there are incompatible annotations in the DAG's components. @@ -554,11 +543,6 @@ def create_execution_info( dict: Dictionary with functions and their arguments for each node in the DAG. The functions are already in topological_sort order. - Raises: - ------ - NonStringAnnotationError: If `verify_annotations` is `True` and the type - annotations are not strings. - """ out = {} for node in nx.lexicographical_topological_sort(dag, key=lexsort_key): @@ -595,11 +579,11 @@ def _create_concatenated_function( return annotation of the concatenated function reflects the requested return type and number of targets (e.g., for two targets returned as a list, the return annotation is a list of their respective type hints). Note that this - is not a valid type annotation and should not be used for type checking. All - annotations must be strings; otherwise, a NonStringAnnotationError is - raised. To ensure string annotations, enclose them in quotes or use "from - __future__ import annotations" at the top of your file. An - AnnotationMismatchError is raised if annotations differ between functions. + is not a valid type annotation and should not be used for type checking. + Annotations need not be strings: non-string annotations (e.g. when "from + __future__ import annotations" is not used, or on Python 3.14+) are + converted to their string representation. An AnnotationMismatchError is + raised if annotations differ between functions. Returns: ------- diff --git a/tests/test_without_string_annotations.py b/tests/test_without_string_annotations.py index b18f0b3..b2f3ac5 100644 --- a/tests/test_without_string_annotations.py +++ b/tests/test_without_string_annotations.py @@ -5,7 +5,9 @@ """ -from dags.dag import FunctionExecutionInfo +import inspect + +from dags.dag import FunctionExecutionInfo, concatenate_functions def test_function_execution_info() -> None: @@ -32,3 +34,35 @@ def f(a: int, b: float) -> float: ) # Annotations should now be strings assert info.annotations == {"a": "int", "b": "float", "return": "float"} + + +def test_concatenate_functions_set_annotations_converts_non_string_annotations() -> ( + None +): + """`set_annotations=True` converts non-string annotations to strings. + + Regression test for the documented behavior: non-string annotations (here + real `int`/`float` type objects, because this module does not use + `from __future__ import annotations`) must be converted to their string + representation rather than raising. See ``NonStringAnnotationError``, which + is reserved but not currently raised. + """ + + def a() -> int: + return 1 + + def b(a: int) -> float: + return a + 1.5 + + combined = concatenate_functions( + {"a": a, "b": b}, + targets="b", + set_annotations=True, + ) + + signature = inspect.signature(combined) + # `a` is a function (target ancestor), so the combined function has no free + # arguments. The propagated return annotation is the string form of `float`. + assert list(signature.parameters) == [] + assert signature.return_annotation == "float" + assert combined() == 2.5