diff --git a/.github/workflows/ci_pip.yml b/.github/workflows/ci_pip.yml index 6ca019dcb..95d52e5bc 100644 --- a/.github/workflows/ci_pip.yml +++ b/.github/workflows/ci_pip.yml @@ -41,7 +41,7 @@ jobs: run: python -m pip install pytest devtools jsonschema requests wget pooch - name: Test core library with pytest - run: python -m pytest tests --ignore tests/tasks + run: python -m pytest tests --ignore tests/tasks --ignore tests/dev tests_tasks: if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} @@ -83,4 +83,4 @@ jobs: key: pooch-cache - name: Test tasks with pytest - run: python -m pytest tests tests/tasks -s --log-cli-level info + run: python -m pytest tests/dev tests/tasks -s --log-cli-level info diff --git a/.github/workflows/ci_poetry.yml b/.github/workflows/ci_poetry.yml index 65479c9dd..9836cfc8d 100644 --- a/.github/workflows/ci_poetry.yml +++ b/.github/workflows/ci_poetry.yml @@ -43,7 +43,7 @@ jobs: run: poetry install --with dev --without docs --no-interaction - name: Test core library with pytest - run: poetry run coverage run -m pytest tests --ignore tests/tasks + run: poetry run coverage run -m pytest tests --ignore tests/tasks --ignore tests/dev - name: Upload coverage data uses: actions/upload-artifact@v3 @@ -96,7 +96,7 @@ jobs: key: pooch-cache - name: Test tasks with pytest - run: poetry run coverage run -m pytest tests/tasks -s --log-cli-level info + run: poetry run coverage run -m pytest tests/dev tests/tasks -s --log-cli-level info - name: Upload coverage data uses: actions/upload-artifact@v3 diff --git a/CHANGELOG.md b/CHANGELOG.md index c3e16455e..b39c4ee1a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ **Note**: Numbers like (\#123) point to closed Pull Requests on the fractal-tasks-core repository. +# 1.0.3 (unreleased) + +* Support JSON-Schema generation for `Enum` task arguments (\#749). +* Make JSON-Schema generation tools more flexible, to simplify testing (\#749). + # 1.0.2 + * Fix bug in plate metadata in MIP task (in the copy_ome_zarr_hcs_plate init function) (\#736). # 1.0.1 diff --git a/fractal_tasks_core/dev/lib_args_schemas.py b/fractal_tasks_core/dev/lib_args_schemas.py index 971de66a8..f3846de68 100644 --- a/fractal_tasks_core/dev/lib_args_schemas.py +++ b/fractal_tasks_core/dev/lib_args_schemas.py @@ -13,9 +13,11 @@ Helper functions to handle JSON schemas for task arguments. """ import logging +import os from collections import Counter from pathlib import Path from typing import Any +from typing import Callable from typing import Optional from docstring_parser import parse as docparse @@ -156,28 +158,71 @@ def _remove_attributes_from_descriptions(old_schema: _Schema) -> _Schema: def create_schema_for_single_task( executable: str, - package: str = "fractal_tasks_core", + package: Optional[str] = "fractal_tasks_core", custom_pydantic_models: Optional[list[tuple[str, str, str]]] = None, + task_function: Optional[Callable] = None, + verbose: bool = False, ) -> _Schema: """ Main function to create a JSON Schema of task arguments + + This function can be used in two ways: + + 1. `task_function` argument is `None`, `package` is set, and `executable` + is a path relative to that package. + 2. `task_function` argument is provided, `executable` is an absolute path + to the function module, and `package` is `None. This is useful for + testing. + """ logging.info("[create_schema_for_single_task] START") - - # Extract the function name. Note: this could be made more general, but for - # the moment we assume the function has the same name as the module) - function_name = Path(executable).with_suffix("").name - logging.info(f"[create_schema_for_single_task] {function_name=}") + if task_function is None: + usage = "1" + # Usage 1 (standard) + if package is None: + raise ValueError( + "Cannot call `create_schema_for_single_task with " + f"{task_function=} and {package=}. Exit." + ) + if os.path.isabs(executable): + raise ValueError( + "Cannot call `create_schema_for_single_task with " + f"{task_function=} and absolute {executable=}. Exit." + ) + else: + usage = "2" + # Usage 2 (testing) + if package is not None: + raise ValueError( + "Cannot call `create_schema_for_single_task with " + f"{task_function=} and non-None {package=}. Exit." + ) + if not os.path.isabs(executable): + raise ValueError( + "Cannot call `create_schema_for_single_task with " + f"{task_function=} and non-absolute {executable=}. Exit." + ) # Extract function from module - task_function = _extract_function( - package_name=package, - module_relative_path=executable, - function_name=function_name, - ) + if usage == "1": + # Extract the function name (for the moment we assume the function has + # the same name as the module) + function_name = Path(executable).with_suffix("").name + # Extract the function object + task_function = _extract_function( + package_name=package, + module_relative_path=executable, + function_name=function_name, + verbose=verbose, + ) + else: + # The function object is already available, extract its name + function_name = task_function.__name__ - logging.info(f"[create_schema_for_single_task] {task_function=}") + if verbose: + logging.info(f"[create_schema_for_single_task] {function_name=}") + logging.info(f"[create_schema_for_single_task] {task_function=}") # Validate function signature against some custom constraints _validate_function_signature(task_function) @@ -190,16 +235,18 @@ def create_schema_for_single_task( schema = _remove_attributes_from_descriptions(schema) # Include titles for custom-model-typed arguments - schema = _include_titles(schema) + schema = _include_titles(schema, verbose=verbose) - # Include descriptions of function arguments + # Include descriptions of function. Note: this function works both + # for usages 1 or 2 (see docstring). function_args_descriptions = _get_function_args_descriptions( package_name=package, - module_relative_path=executable, + module_path=executable, function_name=function_name, + verbose=verbose, ) schema = _insert_function_args_descriptions( - schema=schema, descriptions=function_args_descriptions + schema=schema, descriptions=function_args_descriptions, verbose=verbose ) # Merge lists of fractal-tasks-core and user-provided Pydantic models diff --git a/fractal_tasks_core/dev/lib_descriptions.py b/fractal_tasks_core/dev/lib_descriptions.py index a4c1e7647..e92acea3e 100644 --- a/fractal_tasks_core/dev/lib_descriptions.py +++ b/fractal_tasks_core/dev/lib_descriptions.py @@ -10,8 +10,10 @@ # Zurich. import ast import logging +import os from importlib import import_module from pathlib import Path +from typing import Optional from docstring_parser import parse as docparse @@ -37,23 +39,49 @@ def _sanitize_description(string: str) -> str: def _get_function_docstring( - package_name: str, module_relative_path: str, function_name: str + *, + package_name: Optional[str], + module_path: str, + function_name: str, + verbose: bool = False, ) -> str: """ Extract docstring from a function. + Args: package_name: Example `fractal_tasks_core`. - module_relative_path: Example `tasks/create_ome_zarr.py`. + module_path: + This must be an absolute path like `/some/module.py` (if + `package_name` is `None`) or a relative path like `something.py` + (if `package_name` is not `None`). function_name: Example `create_ome_zarr`. """ - if not module_relative_path.endswith(".py"): - raise ValueError(f"Module {module_relative_path} must end with '.py'") + if not module_path.endswith(".py"): + raise ValueError(f"Module {module_path} must end with '.py'") # Get the function ast.FunctionDef object - package_path = Path(import_module(package_name).__file__).parent - module_path = package_path / module_relative_path + if package_name is not None: + if os.path.isabs(module_path): + raise ValueError( + "Error in _get_function_docstring: `package_name` is not " + "None but `module_path` is absolute." + ) + package_path = Path(import_module(package_name).__file__).parent + module_path = package_path / module_path + else: + if not os.path.isabs(module_path): + raise ValueError( + "Error in _get_function_docstring: `package_name` is None " + "but `module_path` is not absolute." + ) + module_path = Path(module_path) + + if verbose: + logging.info(f"[_get_function_docstring] {function_name=}") + logging.info(f"[_get_function_docstring] {module_path=}") + tree = ast.parse(module_path.read_text()) _function = next( f @@ -66,21 +94,33 @@ def _get_function_docstring( def _get_function_args_descriptions( - package_name: str, module_relative_path: str, function_name: str + *, + package_name: Optional[str], + module_path: str, + function_name: str, + verbose: bool = False, ) -> dict[str, str]: """ Extract argument descriptions from a function. Args: package_name: Example `fractal_tasks_core`. - module_relative_path: Example `tasks/create_ome_zarr.py`. + module_path: + This must be an absolute path like `/some/module.py` (if + `package_name` is `None`) or a relative path like `something.py` + (if `package_name` is not `None`). function_name: Example `create_ome_zarr`. """ # Extract docstring from ast.FunctionDef docstring = _get_function_docstring( - package_name, module_relative_path, function_name + package_name=package_name, + module_path=module_path, + function_name=function_name, + verbose=verbose, ) + if verbose: + logging.info(f"[_get_function_args_descriptions] {docstring}") # Parse docstring (via docstring_parser) and prepare output parsed_docstring = docparse(docstring) @@ -134,7 +174,9 @@ def _get_class_attrs_descriptions( return descriptions -def _insert_function_args_descriptions(*, schema: dict, descriptions: dict): +def _insert_function_args_descriptions( + *, schema: dict, descriptions: dict, verbose: bool = False +): """ Merge the descriptions obtained via `_get_args_descriptions` into the properties of an existing JSON Schema. @@ -154,6 +196,11 @@ def _insert_function_args_descriptions(*, schema: dict, descriptions: dict): else: value["description"] = "Missing description" new_properties[key] = value + if verbose: + logging.info( + "[_insert_function_args_descriptions] " + f"Add {key=}, {value=}" + ) new_schema["properties"] = new_properties logging.info("[_insert_function_args_descriptions] END") return new_schema diff --git a/fractal_tasks_core/dev/lib_signature_constraints.py b/fractal_tasks_core/dev/lib_signature_constraints.py index af62db66f..9a4d7abf6 100644 --- a/fractal_tasks_core/dev/lib_signature_constraints.py +++ b/fractal_tasks_core/dev/lib_signature_constraints.py @@ -34,6 +34,7 @@ def _extract_function( module_relative_path: str, function_name: str, package_name: str = "fractal_tasks_core", + verbose: bool = False, ) -> Callable: """ Extract function from a module with the same name. @@ -42,6 +43,7 @@ def _extract_function( package_name: Example `fractal_tasks_core`. module_relative_path: Example `tasks/create_ome_zarr.py`. function_name: Example `create_ome_zarr`. + verbose: """ if not module_relative_path.endswith(".py"): raise ValueError(f"{module_relative_path=} must end with '.py'") @@ -49,8 +51,20 @@ def _extract_function( Path(module_relative_path).with_suffix("") ) module_relative_path_dots = module_relative_path_no_py.replace("/", ".") - module = import_module(f"{package_name}.{module_relative_path_dots}") - task_function = getattr(module, function_name) + if verbose: + logging.info( + f"Now calling `import_module` for " + f"{package_name}.{module_relative_path_dots}" + ) + imported_module = import_module( + f"{package_name}.{module_relative_path_dots}" + ) + if verbose: + logging.info( + f"Now getting attribute {function_name} from " + f"imported module {imported_module}." + ) + task_function = getattr(imported_module, function_name) return task_function diff --git a/fractal_tasks_core/dev/lib_task_docs.py b/fractal_tasks_core/dev/lib_task_docs.py index 201e82b49..4af6f9ad7 100644 --- a/fractal_tasks_core/dev/lib_task_docs.py +++ b/fractal_tasks_core/dev/lib_task_docs.py @@ -18,19 +18,21 @@ def _get_function_description( - package_name: str, module_relative_path: str, function_name: str + package_name: str, module_path: str, function_name: str ) -> str: """ Extract function description from its docstring. Args: package_name: Example `fractal_tasks_core`. - module_relative_path: Example `tasks/create_ome_zarr.py`. + module_path: Example `tasks/create_ome_zarr.py`. function_name: Example `create_ome_zarr`. """ # Extract docstring from ast.FunctionDef docstring = _get_function_docstring( - package_name, module_relative_path, function_name + package_name=package_name, + module_path=module_path, + function_name=function_name, ) # Parse docstring (via docstring_parser) parsed_docstring = docparse(docstring) @@ -72,7 +74,7 @@ def create_docs_info( # Get function description description = _get_function_description( package_name=package, - module_relative_path=executable, + module_path=executable, function_name=function_name, ) docs_info.append(f"## {function_name}\n{description}\n") diff --git a/fractal_tasks_core/dev/lib_titles.py b/fractal_tasks_core/dev/lib_titles.py index 5cf248bf0..07d8f3259 100644 --- a/fractal_tasks_core/dev/lib_titles.py +++ b/fractal_tasks_core/dev/lib_titles.py @@ -19,7 +19,8 @@ def _include_titles_for_properties( - properties: dict[str, dict] + properties: dict[str, dict], + verbose: bool = False, ) -> dict[str, dict]: """ Scan through properties of a JSON Schema, and set their title when it is @@ -31,16 +32,27 @@ def _include_titles_for_properties( Args: properties: TBD """ + if verbose: + logging.info( + f"[_include_titles_for_properties] Original properties:\n" + f"{properties}" + ) + new_properties = properties.copy() for prop_name, prop in properties.items(): if "title" not in prop.keys(): new_prop = prop.copy() new_prop["title"] = prop_name.title() new_properties[prop_name] = new_prop + if verbose: + logging.info( + f"[_include_titles_for_properties] New properties:\n" + f"{new_properties}" + ) return new_properties -def _include_titles(schema: _Schema) -> _Schema: +def _include_titles(schema: _Schema, verbose: bool = False) -> _Schema: """ Include property titles, when missing. @@ -56,19 +68,39 @@ def _include_titles(schema: _Schema) -> _Schema: """ new_schema = schema.copy() + if verbose: + logging.info("[_include_titles] START") + logging.info(f"[_include_titles] Input schema:\n{schema}") + # Update first-level properties (that is, task arguments) - new_properties = _include_titles_for_properties(schema["properties"]) + new_properties = _include_titles_for_properties( + schema["properties"], verbose=verbose + ) new_schema["properties"] = new_properties + if verbose: + logging.info("[_include_titles] Titles for properties now included.") + # Update properties of definitions if "definitions" in schema.keys(): new_definitions = schema["definitions"].copy() for def_name, def_schema in new_definitions.items(): - new_properties = _include_titles_for_properties( - def_schema["properties"] - ) - new_definitions[def_name]["properties"] = new_properties + if "properties" not in def_schema.keys(): + if verbose: + logging.info( + f"Definition schema {def_name} has no 'properties' " + "key. Skip." + ) + else: + new_properties = _include_titles_for_properties( + def_schema["properties"], verbose=verbose + ) + new_definitions[def_name]["properties"] = new_properties new_schema["definitions"] = new_definitions - logging.info("[_include_titles] END") + if verbose: + logging.info( + "[_include_titles] Titles for definitions properties now included." + ) + logging.info("[_include_titles] END") return new_schema diff --git a/tests/dev/test_create_schema_for_single_task.py b/tests/dev/test_create_schema_for_single_task.py new file mode 100644 index 000000000..1a7b17057 --- /dev/null +++ b/tests/dev/test_create_schema_for_single_task.py @@ -0,0 +1,80 @@ +import json + +import pytest +from devtools import debug +from pydantic.decorator import validate_arguments + +from fractal_tasks_core.dev.lib_args_schemas import ( + create_schema_for_single_task, +) + + +def test_create_schema_for_single_task_usage_1(): + """ + This test reproduces the standard schema-creation scenario, as it's found + when running `create_manifest.py` + """ + schema = create_schema_for_single_task( + executable="tasks/cellpose_segmentation.py", + package="fractal_tasks_core", + verbose=True, + ) + debug(schema) + print(json.dumps(schema, indent=2)) + + +@validate_arguments +def task_function(arg_1: int = 1): + """ + Description + + Args: + arg_1: Description of arg_1. + """ + + +def test_create_schema_for_single_task_usage_2(): + """ + This test reproduces the schema-creation scenario starting from an + existing function, as it's done in tests. + """ + schema = create_schema_for_single_task( + task_function=task_function, + executable=__file__, + package=None, + verbose=True, + ) + debug(schema) + print(json.dumps(schema, indent=2)) + + +def test_create_schema_for_single_task_failures(): + """ + This test reproduces some invalid usage of the schema-creation function + """ + with pytest.raises(ValueError): + create_schema_for_single_task( + task_function=task_function, + executable=__file__, + package="something", + verbose=True, + ) + with pytest.raises(ValueError): + create_schema_for_single_task( + task_function=task_function, + executable="non_absolute/path/module.py", + package=None, + verbose=True, + ) + with pytest.raises(ValueError): + create_schema_for_single_task( + executable="/absolute/path/cellpose_segmentation.py", + package="fractal_tasks_core", + verbose=True, + ) + with pytest.raises(ValueError): + create_schema_for_single_task( + executable="cellpose_segmentation.py", + package=None, + verbose=True, + ) diff --git a/tests/dev/test_enum_arguments.py b/tests/dev/test_enum_arguments.py new file mode 100644 index 000000000..5b1262e97 --- /dev/null +++ b/tests/dev/test_enum_arguments.py @@ -0,0 +1,53 @@ +import json +from enum import Enum + +from devtools import debug +from pydantic.decorator import validate_arguments + +from fractal_tasks_core.dev.lib_args_schemas import ( + create_schema_for_single_task, +) + + +class ColorA(Enum): + RED = "this-is-red" + GREEN = "this-is-green" + + +ColorB = Enum( + "ColorB", + {"RED": "this-is-red", "GREEN": "this-is-green"}, + type=str, +) + + +@validate_arguments +def task_function( + arg_A: ColorA, + arg_B: ColorB, +): + """ + Short task description + + Args: + arg_A: Description of arg_A. + arg_B: Description of arg_B. + """ + pass + + +def test_enum_argument(): + """ + This test only asserts that `create_schema_for_single_task` runs + successfully. Its goal is also to offer a quick way to experiment + with new task arguments and play with the generated JSON Schema, + without re-building the whole fractal-tasks-core manifest. + """ + schema = create_schema_for_single_task( + task_function=task_function, + executable=__file__, + package=None, + verbose=True, + ) + debug(schema) + print(json.dumps(schema, indent=2)) diff --git a/tests/dev/test_extract_function.py b/tests/dev/test_extract_function.py new file mode 100644 index 000000000..1d4bc65ec --- /dev/null +++ b/tests/dev/test_extract_function.py @@ -0,0 +1,25 @@ +from devtools import debug + +from fractal_tasks_core.dev.lib_signature_constraints import _extract_function + + +def test_extract_function(): + """ + This test showcases the use of `_extract_function`. + """ + + fun1 = _extract_function( + module_relative_path="zarr_utils.py", + package_name="fractal_tasks_core.ngff", + function_name="load_NgffImageMeta", + verbose=True, + ) + debug(fun1) + + fun2 = _extract_function( + module_relative_path="ngff.zarr_utils.py", + package_name="fractal_tasks_core", + function_name="load_NgffImageMeta", + verbose=True, + ) + debug(fun2) diff --git a/tests/dev/test_unit_lib_descriptions.py b/tests/dev/test_unit_lib_descriptions.py index 19214c347..c8d2e3088 100644 --- a/tests/dev/test_unit_lib_descriptions.py +++ b/tests/dev/test_unit_lib_descriptions.py @@ -10,19 +10,21 @@ def test_get_function_args_descriptions(): args_descriptions = _get_function_args_descriptions( - "fractal_tasks_core", - "dev/lib_signature_constraints.py", - "_extract_function", + package_name="fractal_tasks_core", + module_path="dev/lib_signature_constraints.py", + function_name="_extract_function", ) debug(args_descriptions) assert args_descriptions.keys() == set( - ("package_name", "module_relative_path", "function_name") + ("package_name", "module_relative_path", "function_name", "verbose") ) def test_get_class_attrs_descriptions(): attrs_descriptions = _get_class_attrs_descriptions( - "fractal_tasks_core", "channels.py", "ChannelInputModel" + package_name="fractal_tasks_core", + module_relative_path="channels.py", + class_name="ChannelInputModel", ) debug(attrs_descriptions) assert attrs_descriptions.keys() == set(("wavelength_id", "label"))