diff --git a/docs/MANIFEST.md b/docs/MANIFEST.md index 662d528..3742ec1 100644 --- a/docs/MANIFEST.md +++ b/docs/MANIFEST.md @@ -57,4 +57,6 @@ A tool declares itself with Python types and a few markers; hosts (Napari, Fiji, `ToolError("code", "message")` gives the person a readable message (`no_result` and `no_match` are shown as notices, not failures); `progress(fraction, "text")` updates the progress bar; `check_cancel()` lets Cancel stop the tool between steps. ## Adding something to the manifest +Misuse of a hint is refused when the app is registered (`DeclarationError`, a message naming the parameter and the fix): `Min`/`Max` on anything but an int or float, a bound that is not a finite number, `Min` above `Max`, `PixelSizeOf` on anything but a float, `Axes` on anything but an Image/Labels input or an image/labels output, the output markers `Name`, `Replace` and `ApplyTo` on an input, and two outputs given the same `Name`. + A feature joins one family, follows the naming pattern (`XxxOf("param")` for links, `XxxOut` for outputs), and is not merged until it has: a row here, a recipe row in `AUTHORING.md`, a line in `check`, a use in the example app, a clear declaration error for misuse, and a test in every host (or a documented fallback in `HOST_FEATURES.md`). diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 20e7081..3f2f7ce 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -17,6 +17,12 @@ Value rules (enforced by the worker, each violation is a `FAILURE` with code `in * A `choice` declared with an `Enum` annotation reaches the tool as the Enum member. * An image/labels input with no pixels is refused (`empty_image`), from a file or from shared memory. * Table inputs (a CSV file, UTF-8, first line is the header) reach the tool as a DataFrame exactly as written: floating point cells are read with every digit (`float_precision="round_trip"`, about twice as slow as pandas' default on very large tables); column names are kept as written, duplicates (`a,a`) and empty names included (pandas' renaming to `a.1` / `Unnamed: 0` is switched off, so a tool must address such columns by position); an empty cell is missing (NaN); the spellings `NaN`, `nan`, `NAN` mean a missing number (a column of numbers, `NaN` and empty cells, or of only `NaN` and empty cells, is a float column with NaN), and the other words pandas calls missing (`NA`, `N/A`, `NULL`, `None`, `null`, ...) stay text, except in a column whose other cells are all numbers, where they are missing numbers; a text column that happens to contain `nan` keeps it as text. A table file that does not exist is refused with `file_not_found`; one that is empty or not a UTF-8 CSV with `unreadable_table`; both messages name the file. +* `integer` and `float` values must be JSON numbers: text (even "3"), lists, objects, NaN and infinity give `invalid_parameter` (`'N' must be an integer, got text`, `'N' must be a number, got a list`, `'N' must be a finite number`); an integer-valued float such as `3.0` is accepted for an `integer`. A number beyond the float range is refused for a `float`. +* `file`, `folder` and `image`/`labels` values must be text paths: anything else gives `invalid_parameter` (`'N' must be a path (text), got a list: the host must send the file or folder path as text`). +* A shared-memory image descriptor must be `{"appose_type": "ndarray", "shm": {"name"}, "shape", "dtype"}` and the block must still exist and be large enough, with a numeric dtype; a stale or malformed one gives `unreadable_image` (`the shared-memory image for 'N' is gone or malformed (...): the host must keep the block alive until the task ends and send {...}`). +* What a tool returns must match its declaration, otherwise the task fails with `bad_return` and a message of the form `the output '' must be , got ` (an array of numbers for image/labels, a DataFrame / dict of lists / list of dicts for table and points, a dict for values, a path for file, text for message, a 3x3 matrix of numbers for affine). A tool with several outputs must return a tuple with one value per output (`tool returned N values, declared M`). An affine with NaN or infinity is refused (`bad_return`). +* Table and points outputs are written as UTF-8 CSV. A NUL character (`\\0`) in a column name or a text cell is refused with `bad_return` (`the table output 'X' has a NUL character in a cell of column 'c': remove it`), identically on every Python version; a lone surrogate or other character that is not UTF-8 is refused with `bad_return` (`... cannot be written as a UTF-8 CSV ...`). Line breaks, quotes and commas in cells and names are quoted and arrive unchanged. +* Image outputs: an empty array is `bad_return` (`image is empty (shape ...)`); complex, object, text and bytes arrays are `unsupported_dtype`; an integer image beyond 16 bits that float32 cannot hold exactly (more than 24 significant bits, for example a uint64 2**64-1) is refused with `unsupported_dtype` on every machine, never stored rounded or kept as 64-bit. * Image outputs: integers are never changed in value (wide integers that fit 16 bits are stored as 16-bit; others must be exact as 32-bit float or the result fails with `unsupported_dtype`); floating point values are stored as float32 (about 7 significant digits); float values outside the float32 range fail. Input `type`: `string integer float boolean choice image labels table file folder`. diff --git a/docs/REGRESSION_LEDGER.md b/docs/REGRESSION_LEDGER.md index eb08b45..b8286d2 100644 --- a/docs/REGRESSION_LEDGER.md +++ b/docs/REGRESSION_LEDGER.md @@ -17,6 +17,13 @@ How to add one: [TESTING.md](TESTING.md#the-ledger). | 2026-10-08 | F3: duplicate and empty column names of a Table input are renamed (`a.1`, `Unnamed: 0`) | `pd.read_csv` header mangling | this PR | `tests/test_table_input.py::TableInput.test_duplicate_and_empty_names_are_kept`; also `test_roundtrip_matrix.py::Matrix.test_worker_protocol` | | 2026-10-08 | F4: a missing, empty or binary table file fails with `FileNotFoundError` / `EmptyDataError` / `UnicodeDecodeError` | no `file_not_found` / unreadable check for tables as for images | this PR | `tests/test_table_input.py::TableInput.test_unreadable_files_are_tool_errors`; also `test_roundtrip_matrix.py::Matrix.test_worker_protocol` | | 2026-10-08 | F7: table text cells such as `NA`, `null`, `None` arrive as NaN | pandas default NA values | this PR | `tests/test_table_input.py::TableInput.test_na_words_stay_text_unless_numeric`; also `test_roundtrip_matrix.py::Matrix.test_worker_protocol` | +| 2026-10-08 | F1: a number parameter given text, a list, NaN or infinity fails with `[ValueError]`/`[TypeError]`/`[OverflowError]` instead of `invalid_parameter` | `convert._load_one` calls `int(value)` / `float(value)` before it checks anything | this PR | `test_error_codes.py::NumbersMustBeNumbers.test_integer_given_something_else_is_invalid_parameter_naming_what_arrived`, `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_int/word` and 12 more), `test_roundtrip_properties.py::Properties.test_only_toolerror_integer`, `test_roundtrip_properties.py::Properties.test_only_toolerror_float` | +| 2026-10-08 | F5: a tool that returns the wrong kind of value for an output gets a raw Python exception code | `build_results` and its writers trust the shape and type of what a tool returns | this PR | `test_error_codes.py::WrongReturns.test_each_kind_of_wrong_value_names_the_output_and_what_was_expected`, `test_roundtrip_failures.py::WhatAToolReturns.test_the_wrong_type_is_reported_with_a_tool_error_code` | +| 2026-10-08 | F6: an Affine output with NaN or infinity is delivered as null | `_as_3x3` does not check finiteness (points and shapes do) | this PR | `test_error_codes.py::WrongReturns.test_affine_with_nan_or_infinity_is_refused`, `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_affine/invalid/nan-cell`) | +| 2026-10-08 | F11, F12: a malformed shared-memory descriptor, or a non-text file/folder value, raises KeyError / FileNotFoundError / TypeError | no validation before `SharedMemory(...)` and `Path(...)` | this PR | `test_error_codes.py::SharedMemoryDescriptors.test_a_block_that_is_gone_or_a_descriptor_with_missing_or_wrong_fields`, `test_error_codes.py::PathsMustBeText.test_file_folder_and_image_given_a_non_text_value`, `test_roundtrip_properties.py::Properties.test_only_toolerror_image_shared_memory`, `test_roundtrip_properties.py::Properties.test_only_toolerror_file_value` | +| 2026-10-08 | F15: an empty, complex, object or text array is written as an image result (or crashes with KeyError) | `portable_dtype` and `_write_one` do not refuse what no host can open | this PR | `test_error_codes.py::ImagesNoHostCanOpen.test_complex_object_and_text_arrays_are_unsupported_dtype`, `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`make_array/*`) | +| 2026-10-08 | F16: hints on the wrong kind of parameter, `Min(5), Max(1)`, output markers on inputs and duplicate output names are accepted and ignored | `introspection` validates only some misuses | this PR | `test_error_codes.py::HintMisuse.test_bounds_on_text_and_an_empty_range`, `test_error_codes.py::HintMisuse.test_duplicate_output_names`, `test_roundtrip_hints.py::InvalidCombinations.test_every_invalid_combination_is_refused_with_a_message_that_names_the_culprit` | +| 2026-10-08 | F20: on ARM (Apple silicon) a uint64 image holding 2**64-1 is stored as float32 2**64 instead of being refused | `portable_dtype` checks exactness by casting the float32 back to uint64, an out-of-range cast that is undefined: x86 gives 0 (refused), ARM saturates (accepted) | this PR | `test_error_codes.py::Float32Exactness.test_uint64_beyond_float32_is_refused_with_unsupported_dtype_on_every_cpu` (numpy warns about the out-of-range cast on x86, the test turns that into a failure), `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_image/uint64/beyond-float32`; found by the macOS job of CI) | ## Found by the round-trip matrix, fix PRs pending @@ -26,18 +33,11 @@ a finding deletes its entries there and moves its row up to the table above. | Date | Symptom | Root cause | Fix PR | Guarding test | |---|---|---|---|---| -| 2026-10-08 | F1: a number parameter given text, a list, NaN or infinity fails with `[ValueError]`/`[TypeError]`/`[OverflowError]` instead of `invalid_parameter` | `convert._load_one` calls `int(value)` / `float(value)` before it checks anything | pending | `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_int/word` and 12 more), `test_roundtrip_properties.py::Properties.test_only_toolerror_integer`, `test_roundtrip_properties.py::Properties.test_only_toolerror_float` | -| 2026-10-08 | F5: a tool that returns the wrong kind of value for an output gets a raw Python exception code | `build_results` and its writers trust the shape and type of what a tool returns | pending | `test_roundtrip_failures.py::WhatAToolReturns.test_the_wrong_type_is_reported_with_a_tool_error_code` | -| 2026-10-08 | F6: an Affine output with NaN or infinity is delivered as null | `_as_3x3` does not check finiteness (points and shapes do) | pending | `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_affine/invalid/nan-cell`) | | 2026-10-08 | F8, F9: the notebook form clamps numbers beyond 10**9 and cannot leave an optional parameter unset (the tool receives 0, empty text, False, the first choice) | `BoundedIntText` / `Text` / `Dropdown` controls always hold a value | this PR | `test_roundtrip_matrix.py::Matrix.test_notebook_form` (`echo_optional_*/omitted`, `echo_int/ok/2147483648`), `test_notebook_unset_and_range.py` | | 2026-10-08 | F10: a copied Windows command line changes a text with backslashes before a quote | `command.quote(windows=True)` does not double backslashes in front of a quote or at the end of the text | this PR: `command._quote_windows` follows the C runtime rules; vectors shared with the Groovy hosts in `tests/quote_vectors.json` | `test_quote_vectors.py::QuoteVectors.test_quote_gives_the_expected_text_for_both_shells`, `test_roundtrip_command.py::QuotingHelper.test_windows_quoting_keeps_every_text_one_argument`, `test_roundtrip_command.py::QuotingHelper.test_windows_quoting_survives_the_real_process_start` (the Windows jobs of CI show it on a real process start) | -| 2026-10-08 | F11, F12: a malformed shared-memory descriptor, or a non-text file/folder value, raises KeyError / FileNotFoundError / TypeError | no validation before `SharedMemory(...)` and `Path(...)` | pending | `test_roundtrip_properties.py::Properties.test_only_toolerror_image_shared_memory`, `test_roundtrip_properties.py::Properties.test_only_toolerror_file_value` | | 2026-10-08 | F13: the worker destroys the host's shared-memory block when it exits (and says so in crash messages) | Python's resource tracker unlinks every block the worker attached to | this PR: `convert._attach_shared_memory` attaches without tracking (`track=False` on Python 3.13+, unregister after attaching before) | `test_roundtrip_shared_memory.py::SharedMemoryOwnership.test_the_block_and_the_stderr_are_left_alone`, `test_lifecycle_fixes.py::SharedMemoryAttach.test_a_process_that_only_attached_leaves_the_block_and_its_stderr_alone` | | 2026-10-08 | F21: on Windows `labconstrictor-tools register` sometimes fails with `PermissionError [WinError 5]` from `os.replace` (seen once on CI, Windows 3.12) | `registry._write_atomic` replaces the file once; Windows refuses while another process (scanner, indexer) briefly holds the freshly written temporary file | this PR: `registry._replace` retries `os.replace` on Windows (`REPLACE_ATTEMPTS`, `REPLACE_PAUSE_S`, each retry logged, then an error that says why); the test harness no longer retries | `test_lifecycle_fixes.py::ReplaceRetry` (simulated refusals), `test_roundtrip_matrix.py::Matrix.test_command_line` (intermittent on Windows before) | | 2026-10-08 | F14: `labels_to_shapes` drops single-pixel objects with the default settings | Douglas-Peucker with 0.5 px collapses a one-pixel diamond to fewer than 4 points, which is then skipped | this PR | `test_roundtrip_geometry.py::Outlines.test_default_simplification_never_drops_a_label`, `test_shapes_every_label.py` | -| 2026-10-08 | F15: an empty, complex, object or text array is written as an image result (or crashes with KeyError) | `portable_dtype` and `_write_one` do not refuse what no host can open | pending | `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`make_array/*`) | -| 2026-10-08 | F16: hints on the wrong kind of parameter, `Min(5), Max(1)`, output markers on inputs and duplicate output names are accepted and ignored | `introspection` validates only some misuses | pending | `test_roundtrip_hints.py::InvalidCombinations.test_every_invalid_combination_is_refused_with_a_message_that_names_the_culprit` | | 2026-10-08 | F17: `WorkerProcess.task()` on a dead worker raises BrokenPipeError | `_send` writes to a closed pipe and the task is already registered | this PR: `WorkerProcess.task` ends the task as CRASHED with the worker's last words and does not keep it | `test_roundtrip_failures.py::Crashes.test_asking_a_dead_worker_for_a_task_gives_a_crashed_task`, `test_lifecycle_fixes.py::DeadWorker` | | 2026-10-08 | F18: POSIX: the processes a tool started survive the worker when the host goes away | the worker leaves without killing its own process group (Windows has the job object) | this PR: `Worker._end_process_group` makes the worker lead its own process group at start (`_lead_own_process_group`: Java hosts cannot start it in one) and kills that group when it leaves (stdin closed) or gives up on an orphaned tool | `test_roundtrip_failures.py::HangsAndCancel.test_children_die_when_the_host_goes_away`, `test_lifecycle_fixes.py::ProcessGroup` | | 2026-10-08 | F19: `progress(0.29)` is shown as 28 % | `int(100 * fraction)` truncates | this PR: `worker._whole_percent` rounds half up | `test_roundtrip_failures.py::WhatAToolReturns.test_every_whole_percent_arrives_as_itself`, `test_lifecycle_fixes.py::Progress` | -| 2026-10-08 | F20: on ARM (Apple silicon) a uint64 image holding 2**64-1 is stored as float32 2**64 instead of being refused | `portable_dtype` checks exactness by casting the float32 back to uint64, an out-of-range cast that is undefined: x86 gives 0 (refused), ARM saturates (accepted) | pending | `test_roundtrip_matrix.py::Matrix.test_worker_protocol` (`echo_image/uint64/beyond-float32`; found by the macOS job of CI) | diff --git a/labconstrictor_tools/convert.py b/labconstrictor_tools/convert.py index 34eb489..7ac8863 100644 --- a/labconstrictor_tools/convert.py +++ b/labconstrictor_tools/convert.py @@ -6,10 +6,12 @@ numpy/pandas/tifffile are imported lazily, only when a tool actually uses those types. """ +import csv import logging import math import os import sys +from collections.abc import Mapping from pathlib import Path from typing import Any @@ -22,6 +24,45 @@ INT16_MIN, INT16_MAX = -32768, 32767 SHARED_MEMORY_TRACK_PARAMETER_FROM = (3, 13) # SharedMemory(track=False) exists from this Python version on DEFAULT_AXES = {2: "YX", 3: "ZYX", 4: "CZYX"} +IMAGE_DTYPE_KINDS = ( + "biuf" # bool, signed/unsigned integer, float: the only element kinds a TIFF that hosts can open holds +) +SHARED_MEMORY_DTYPE_KINDS = ( + "biufc" # what a host may put in a shared-memory block (complex is read, the tool decides) +) +AFFINE_SIZE = 3 +_PLAIN_TYPE_NAMES = { + str: "text", + bytes: "bytes", + list: "a list", + tuple: "a list", + dict: "an object", + bool: "true/false", + int: "an integer", + float: "a number", + type(None): "nothing (None)", +} + + +def _describe_type(value: Any) -> str: + """The kind of `value` in the words a person uses, for error messages ('text', 'a list', ...).""" + return _PLAIN_TYPE_NAMES.get(type(value), "a value of type %s" % type(value).__name__) + + +def _is_json_number(value: Any) -> bool: + """True for a JSON number (int or float, never a boolean): the only thing an integer/float parameter accepts.""" + return isinstance(value, (int, float)) and not isinstance(value, bool) + + +def _path_text(label: str, value: Any) -> Path: + """The path a host sent: it must be text (a number, list or object is a mistake of the host, not a file name).""" + if not isinstance(value, str): + raise ToolError( + "invalid_parameter", + "'%s' must be a path (text), got %s: the host must send the file or folder path as text" + % (label, _describe_type(value)), + ) + return Path(value) def load_inputs(schema: ToolSchema, inputs: dict[str, Any], fn: Any = None) -> dict[str, Any]: @@ -58,7 +99,7 @@ def _restore_enums(fn: Any, kwargs: dict[str, Any]) -> dict[str, Any]: def _load_one(param: ParamSchema, value: Any) -> Any: kind, name = param["type"], param["name"] if kind in ("image", "labels"): - array = _read_image(value) + array = _read_image(value, param["label"]) if array.size == 0: # a file is checked while reading; shared memory is checked here raise ToolError( "empty_image", "'%s' contains no image data (shape %s)" % (param["label"], tuple(array.shape)) @@ -67,20 +108,32 @@ def _load_one(param: ParamSchema, value: Any) -> Any: if kind == "table": return _read_table(param, value) if kind == "file": - return Path(value) + return _path_text(param["label"], value) if kind == "folder": - path = Path(value) + path = _path_text(param["label"], value) if not path.is_dir(): raise ToolError("folder_not_found", "'%s': folder not found: %s" % (param["label"], path)) return path if kind == "integer": - if isinstance(value, bool) or int(value) != value: + if not _is_json_number(value): + raise ToolError( + "invalid_parameter", + "'%s' must be an integer, got %s" % (param["label"], _describe_type(value)), + ) + if isinstance(value, float) and not (math.isfinite(value) and value.is_integer()): raise ToolError("invalid_parameter", "'%s' must be an integer" % param["label"]) return _check_range(param, int(value)) if kind == "float": if isinstance(value, bool): raise ToolError("invalid_parameter", "'%s' must be a number, not true/false" % param["label"]) - number = float(value) + if not _is_json_number(value): + raise ToolError( + "invalid_parameter", "'%s' must be a number, got %s" % (param["label"], _describe_type(value)) + ) + try: + number = float(value) + except OverflowError: # an integer beyond the float range + number = math.inf if not math.isfinite(number): raise ToolError("invalid_parameter", "'%s' must be a finite number" % param["label"]) return _check_range(param, number) @@ -155,8 +208,8 @@ def _read_table(param: ParamSchema, value: Any) -> Any: """ import pandas as pd - path = Path(value) label = param["label"] + path = _path_text(label, value) if not path.is_file(): raise ToolError("file_not_found", "'%s': table file not found: %s" % (label, path)) try: @@ -211,10 +264,10 @@ def is_text(column: int) -> bool: ) from error -def _read_image(value: Any) -> Any: +def _read_image(value: Any, label: str = "image") -> Any: if isinstance(value, dict) and value.get("appose_type") == "ndarray": - return _ndarray_from_shared_memory(value) - path = Path(value) + return _ndarray_from_shared_memory(value, label) + path = _path_text(label, value) if not path.is_file(): raise ToolError("file_not_found", "image file not found: %s" % path) try: @@ -271,21 +324,59 @@ def _attach_shared_memory(name: str) -> Any: resource_tracker.register = register -def _ndarray_from_shared_memory(descriptor: dict[str, Any]) -> Any: +def _ndarray_from_shared_memory(descriptor: dict[str, Any], label: str = "image") -> Any: + """Copy the image out of the host's shared-memory block. A block that is gone (the host freed it, a stale descriptor) or a + descriptor that is incomplete or inconsistent is `unreadable_image` with a message that says what the host must send. + """ import numpy as np - shm = _attach_shared_memory(descriptor["shm"]["name"]) try: - view = np.ndarray(tuple(descriptor["shape"]), dtype=descriptor["dtype"], buffer=shm.buf) + name = descriptor["shm"]["name"] + dtype = np.dtype(descriptor["dtype"]) + if dtype.kind not in SHARED_MEMORY_DTYPE_KINDS: + raise ValueError("dtype %s is not a number type" % dtype) + shape = tuple(descriptor["shape"]) + shm = _attach_shared_memory(name) + except (KeyError, TypeError, ValueError, OSError) as error: + logging.getLogger("labconstrictor.convert").error( + "unusable shared-memory descriptor for %s", label, exc_info=True + ) + raise ToolError("unreadable_image", _shared_memory_message(label, error)) from error + try: + view = np.ndarray(shape, dtype=dtype, buffer=shm.buf) return view.copy() + except (TypeError, ValueError) as error: # shape/dtype larger than the block, or a negative dimension + raise ToolError("unreadable_image", _shared_memory_message(label, error)) from error finally: shm.close() +def _shared_memory_message(label: str, error: Exception) -> str: + """Why a shared-memory image cannot be read and what the host must do (keep the block until the task ends, send all fields).""" + return ( + "the shared-memory image for '%s' is gone or malformed (%s: %s): the host must keep the block alive until the task " + 'ends and send {"appose_type": "ndarray", "shm": {"name": ...}, "shape": [...], "dtype": ...}' + % (label, type(error).__name__, error) + ) + + def build_results(schema: ToolSchema, returned: Any, job_dir: str | Path) -> list[Result]: - """Turn the tool's return value into the list of typed results declared by the schema.""" + """Turn the tool's return value into the list of typed results declared by the schema. Anything that is not what the + declaration promised is `bad_return` with a message that names the output and what was expected, never a raw exception. + """ outputs = schema["outputs"] - values = [returned] if len(outputs) == 1 else list(returned or []) + if len(outputs) == 1: + values = [returned] + elif returned is None: + values = [] + elif isinstance(returned, (tuple, list)): + values = list(returned) + else: + raise ToolError( + "bad_return", + "tool returned %s, declared %d outputs: return a tuple with one value per output" + % (_describe_type(returned), len(outputs)), + ) if len(outputs) != len(values): raise ToolError("bad_return", "tool returned %d values, declared %d" % (len(values), len(outputs))) return [ @@ -293,32 +384,109 @@ def build_results(schema: ToolSchema, returned: Any, job_dir: str | Path) -> lis ] # lengths checked just above (strict= needs 3.10) +def _bad_return(kind: str, name: str, expected: str, value: Any) -> ToolError: + """The one wording of a wrong kind of returned value: which output, what it must be, what the tool returned.""" + return ToolError( + "bad_return", + "the %s output '%s' must be %s, got %s" % (kind, name, expected, _describe_type(value)), + ) + + +def _as_frame(kind: str, name: str, value: Any) -> Any: + """A DataFrame, a dict of equal-length lists/arrays, or a list of dicts, as a DataFrame; anything else is `bad_return`. + The shape is checked here, before pandas sees it: pandas' own errors differ between versions (AttributeError, TypeError, + ValueError) and some mixed lists are accepted by one version and crash another.""" + import numpy as np + import pandas as pd + + expected = "a DataFrame, a dict of lists or a list of dicts" + if isinstance(value, pd.DataFrame): + return value + column_types = (list, tuple, np.ndarray, pd.Series) + if isinstance(value, Mapping): + columns = list(value.values()) + if not all(isinstance(c, column_types) and getattr(c, "ndim", 1) == 1 for c in columns): + raise _bad_return(kind, name, expected + " (every column must be a 1-D list)", value) + if len({len(c) for c in columns}) > 1: + raise ToolError( + "bad_return", + "the %s output '%s' must be %s: the columns have different lengths" % (kind, name, expected), + ) + elif isinstance(value, (list, tuple)): + if not all(isinstance(row, Mapping) for row in value): + raise _bad_return(kind, name, expected + " (every row must be a dict)", value) + else: + raise _bad_return(kind, name, expected, value) + try: + return pd.DataFrame(value) + except (TypeError, ValueError, AttributeError) as error: # whatever the pandas version objects to + raise ToolError( + "bad_return", "the %s output '%s' must be %s (%s)" % (kind, name, expected, error) + ) from error + + +NUL = "\x00" # the CSV writer of Python 3.10 cannot write it and 3.11+ writes it silently: refused identically everywhere + + +def _write_csv(kind: str, name: str, frame: Any, path: Path) -> None: + """Write a table or points result as UTF-8 CSV after refusing what some Python versions cannot write and others would write + wrongly: a NUL character in a column name or a text cell (checked per column, vectorised), and a lone surrogate (not UTF-8). + """ + for column in frame.columns: + if NUL in str(column): + raise ToolError( + "bad_return", + "the %s output '%s' has a NUL character in the name of column %r: remove it" + % (kind, name, str(column)), + ) + for position, column in enumerate(frame.columns): + cells = frame.iloc[:, position] # by position: duplicate column names are allowed + if cells.dtype.kind in "OUS" or str(cells.dtype) in ("string", "str"): + if cells.dropna().astype(str).str.contains(NUL, regex=False).any(): + raise ToolError( + "bad_return", + "the %s output '%s' has a NUL character in a cell of column %r: remove it" + % (kind, name, str(column)), + ) + try: + frame.to_csv(path, index=False, encoding="utf-8") + except (UnicodeEncodeError, csv.Error) as error: + raise ToolError( + "bad_return", + "the %s output '%s' cannot be written as a UTF-8 CSV (%s): remove the character it names (a lone surrogate or control " + "character) from the column names and cells" % (kind, name, error), + ) from error + + def _write_one(out: OutputSchema, value: Any, job_dir: Path) -> Result: kind, name = out["type"], out["name"] if kind in ("image", "labels"): - import numpy as np import tifffile - array = portable_dtype(np.asarray(value), name) + array = portable_dtype(_as_array(kind, name, value), name) path = job_dir / (name + ".tif") tifffile.imwrite(path, array) axes = out.get("axes") or DEFAULT_AXES.get(array.ndim, "") return {"type": kind, "name": name, "path": str(path), "axes": axes} if kind == "table": - import pandas as pd - - frame = value if isinstance(value, pd.DataFrame) else pd.DataFrame(value) + frame = _as_frame("table", name, value) path = job_dir / (name + ".csv") - frame.to_csv(path, index=False) + _write_csv("table", name, frame, path) return {"type": "table", "name": name, "path": str(path)} if kind == "file": + if not isinstance(value, (str, os.PathLike)): + raise _bad_return("file", name, "a path (pathlib.Path or text)", value) return {"type": "file", "name": name, "path": str(value)} if kind == "values": - return {"type": "values", "name": name, "values": {str(k): _plain(v) for k, v in dict(value).items()}} + if not isinstance(value, Mapping): + raise _bad_return("values", name, "a dict of JSON-able values", value) + return {"type": "values", "name": name, "values": {str(k): _plain(v) for k, v in value.items()}} if kind == "affine": - return {"type": "affine", "name": name, "matrix_yx": _as_3x3(value), **out.get("display", {})} + return {"type": "affine", "name": name, "matrix_yx": _as_3x3(value, name), **out.get("display", {})} if kind == "message": - text = str(value).strip() + if not isinstance(value, str): + raise _bad_return("message", name, "text", value) + text = value.strip() if not text: raise ToolError("bad_return", "the message output '%s' is empty" % name) return {"type": "message", "name": name, "text": text} @@ -334,7 +502,7 @@ def _write_points(name: str, value: Any, job_dir: Path, display: dict[str, str]) import numpy as np import pandas as pd - frame = value if isinstance(value, pd.DataFrame) else pd.DataFrame(value) + frame = _as_frame("points", name, value) missing = [c for c in ("y", "x") if c not in frame.columns] if missing: raise ToolError( @@ -352,7 +520,7 @@ def _write_points(name: str, value: Any, job_dir: Path, display: dict[str, str]) others = [c for c in frame.columns if c not in ("y", "x")] frame = frame[["y", "x"] + others] path = job_dir / (name + ".csv") - frame.to_csv(path, index=False) + _write_csv("points", name, frame, path) return { "type": "points", "name": name, @@ -485,17 +653,60 @@ def _plain(value: Any) -> Any: return value -def _as_3x3(matrix: Any) -> list[list[float]]: +def _as_array(kind: str, name: str, value: Any) -> Any: + """The tool's image/labels result as an ndarray; text, nothing, an object or a ragged list is `bad_return`.""" + import numpy as np + + if isinstance(value, (str, bytes, dict)) or value is None: + raise _bad_return(kind, name, "an array of numbers", value) + try: + return np.asarray(value) + except (TypeError, ValueError) as error: # ragged nested lists + raise ToolError( + "bad_return", "the %s output '%s' must be an array of numbers (%s)" % (kind, name, error) + ) from error + + +def _as_3x3(matrix: Any, name: str = "affine") -> list[list[float]]: + """A 2x3 or 3x3 matrix of finite numbers as a 3x3 list; anything else is `bad_return` naming the output.""" import numpy as np - m = np.asarray(matrix, float) - if m.shape == (2, 3): + try: + m = np.asarray(matrix, float) + except (TypeError, ValueError) as error: + raise ToolError( + "bad_return", "the affine output '%s' must be a 3x3 matrix of numbers (%s)" % (name, error) + ) from error + if m.shape == (2, AFFINE_SIZE): m = np.vstack([m, [0, 0, 1]]) - if m.shape != (3, 3): + if m.shape != (AFFINE_SIZE, AFFINE_SIZE): raise ToolError("bad_return", "affine must be 3x3 (got %s)" % (m.shape,)) + if not np.isfinite(m).all(): + raise ToolError( + "bad_return", + "the affine output '%s' has a value that is not a finite number (NaN or infinity): a host cannot apply it" + % name, + ) return m.tolist() +_DTYPE_KIND_NAMES = {"c": "complex numbers", "O": "Python objects", "U": "text", "S": "bytes", "V": "records"} + + +def _float32_holds_exactly(widened: Any, original: Any) -> bool: + """True when every integer of `original` equals its float32 `widened`. The back-conversion is only done for values inside + the integer type's range: casting a float outside it is undefined (x86 gives 0, ARM saturates), which once let a uint64 + 2**64-1 pass as float32 2**64.""" + import numpy as np + + info = np.iinfo(original.dtype) + values = widened.astype(np.float64) # exact: every float32 is a float64 + upper_exclusive = 2.0 ** (info.bits - (1 if info.min < 0 else 0)) + if not ((values >= float(info.min)) & (values < upper_exclusive)).all(): + return False + return bool(np.array_equal(widened.astype(original.dtype), original)) + + def portable_dtype(array: Any, what: str = "image") -> Any: """Cast to a dtype every host can open (ImageJ's TIFF reader has no float64, int64, uint32/64, bool or int8). Integers are never changed in value: wide integers that fit 16 bits are narrowed, anything else must be exactly @@ -504,6 +715,18 @@ def portable_dtype(array: Any, what: str = "image") -> Any: import numpy as np kind, size = array.dtype.kind, array.dtype.itemsize + if array.size == 0: + raise ToolError( + "bad_return", + "%s is empty (shape %s): an image needs at least one pixel in every dimension" + % (what, tuple(array.shape)), + ) + if kind not in IMAGE_DTYPE_KINDS: + raise ToolError( + "unsupported_dtype", + "%s holds %s values (%s): hosts can only open numbers; return a numeric array (integers or floats)" + % (what, array.dtype, _DTYPE_KIND_NAMES.get(kind, "not numbers")), + ) if kind == "b": return array.astype(np.uint8) if kind == "f": @@ -521,15 +744,13 @@ def portable_dtype(array: Any, what: str = "image") -> Any: return array if kind == "i" and size == 1: return array.astype(np.int16) - if array.size == 0: - return array.astype(np.uint16) low, high = int(array.min()), int(array.max()) if low >= 0 and high <= UINT16_MAX: return array.astype(np.uint16) if low >= INT16_MIN and high <= INT16_MAX: return array.astype(np.int16) widened = array.astype(np.float32) - if np.array_equal(widened.astype(array.dtype), array): + if _float32_holds_exactly(widened, array): return widened raise ToolError( "unsupported_dtype", diff --git a/labconstrictor_tools/introspection.py b/labconstrictor_tools/introspection.py index 88bb09f..ba0c704 100644 --- a/labconstrictor_tools/introspection.py +++ b/labconstrictor_tools/introspection.py @@ -12,6 +12,13 @@ PROTOCOL = 1 SCALARS = {str: "string", int: "integer", float: "float", bool: "boolean"} +OUTPUT_ONLY_MARKERS = ( + T.Name, + T.Replace, + T.ApplyTo, +) # these describe a result: on an input they would be ignored +AXES_ON_INPUT = "parameter %r: Axes describes the dimensions of an Image or Labels input and this parameter is not one: remove it" +IMAGE_OUTPUT_TYPES = ("image", "labels") # the output types an Axes hint describes class DeclarationError(Exception): @@ -105,6 +112,7 @@ def _param(name: str, p: inspect.Parameter, hint: Any, argdoc: dict[str, str]) - d["default"] = p.default.value if isinstance(p.default, enum.Enum) else p.default _add_type_fields(d, name, p, hint, base, meta, has_default) _check_default(name, d) + _check_number_hints(d, name, meta) if d["type"] in ("integer", "float"): _add_number_fields(d, name, meta) _add_display_fields(d, name, meta, argdoc) @@ -128,9 +136,17 @@ def _add_type_fields( has_default: bool, ) -> None: """The parameter's `type` (and its image/choice specifics), from the annotation.""" + for marker in OUTPUT_ONLY_MARKERS: + if _m(meta, marker): + raise DeclarationError( + "parameter %r: %s describes a result, not an input: put it on the return annotation or remove it" + % (name, marker.__name__) + ) if base in T.INPUT_TYPES: d["type"] = T.INPUT_TYPES[base] if _m(meta, T.Axes): + if d["type"] not in IMAGE_OUTPUT_TYPES: + raise DeclarationError(AXES_ON_INPUT % name) d["axes"] = _m(meta, T.Axes).value if _m(meta, T.PickChannel) and _m(meta, T.PickChannel).value: if d["type"] != "image": @@ -141,6 +157,8 @@ def _add_type_fields( % name ) d["pick_channel"] = True + elif _m(meta, T.Axes): + raise DeclarationError(AXES_ON_INPUT % name) elif get_origin(base) is Literal: ch = list(get_args(base)) d["type"] = "choice" @@ -160,6 +178,40 @@ def _add_type_fields( raise DeclarationError("parameter %r: unsupported annotation %r" % (name, hint)) +def _check_number_hints(d: dict[str, Any], name: str, meta: list[Any]) -> None: + """Min / Max / PixelSizeOf say something about a number: on any other parameter they would be silently ignored, and a + bound that is not a number or a Min above Max can never be satisfied, so each is refused with what to change. + """ + if d["type"] not in ("integer", "float"): + for marker in (T.Min, T.Max): + if _m(meta, marker): + raise DeclarationError( + "parameter %r: %s applies to an int or float parameter; this one is of type %s: remove it" + % (name, marker.__name__, d["type"]) + ) + if _m(meta, T.PixelSizeOf) and d["type"] != "float": + raise DeclarationError( + "parameter %r: PixelSizeOf applies to a float parameter (the calibration in um/px); this one is of type %s: remove it" + % (name, d["type"]) + ) + bounds = {} + for marker in (T.Min, T.Max): + found = _m(meta, marker) + if not found: + continue + bound = found.value + if isinstance(bound, bool) or not isinstance(bound, (int, float)) or not math.isfinite(bound): + raise DeclarationError( + "parameter %r: %s(%r) must be a finite number" % (name, marker.__name__, bound) + ) + bounds[marker.__name__] = bound + if "Min" in bounds and "Max" in bounds and bounds["Min"] > bounds["Max"]: + raise DeclarationError( + "parameter %r: Min(%r) is above Max(%r), so no value is allowed: swap them or widen the range" + % (name, bounds["Min"], bounds["Max"]) + ) + + def _add_number_fields(d: dict[str, Any], name: str, meta: list[Any]) -> None: """Min / Max / Unit / PixelSizeOf of a numeric parameter, checking the default against the bounds.""" for key, cls in (("minimum", T.Min), ("maximum", T.Max)): @@ -251,19 +303,36 @@ def _outputs(ret: Any, tool_id: str) -> list[OutputSchema]: ) o["replace"] = True if _m(m, T.Axes): + if o["type"] not in IMAGE_OUTPUT_TYPES: + raise DeclarationError( + "tool %r output %d: Axes describes an image or labels output; this one is of type %s: remove it" + % (tool_id, i, o["type"]) + ) o["axes"] = _m(m, T.Axes).value + o["explicit_name"] = bool(_m(m, T.Name)) a = _m(m, T.ApplyTo) if a: o["display"] = {"apply_to": a.source, **({"relative_to": a.target} if a.target else {})} outs.append(o) + explicit = [o["name"] for o in outs if o.pop("explicit_name")] + if len(set(explicit)) != len(explicit): + duplicated = sorted({n for n in explicit if explicit.count(n) > 1}) + raise DeclarationError( + "tool %r: two outputs are named %s: give each output its own Name(...)" + % (tool_id, ", ".join(repr(n) for n in duplicated)) + ) seen: dict[str, int] = {} for o in outs: # unnamed outputs of the same type get a numeric suffix seen[o["name"]] = seen.get(o["name"], 0) + 1 if seen[o["name"]] > 1: o["name"] += str(seen[o["name"]]) names = [o["name"] for o in outs] - if len(set(names)) != len(names): - raise DeclarationError("tool %r: duplicate output names %s" % (tool_id, names)) + if len(set(names)) != len( + names + ): # an unnamed output's generated name ("image2") equals a name the author chose + raise DeclarationError( + "tool %r: duplicate output names %s: give the outputs different Name(...)s" % (tool_id, names) + ) return cast(list[OutputSchema], outs) diff --git a/tests/roundtrip_known_failures.py b/tests/roundtrip_known_failures.py index 6256c80..485cdfb 100644 --- a/tests/roundtrip_known_failures.py +++ b/tests/roundtrip_known_failures.py @@ -30,76 +30,7 @@ class Finding: SCHEMA = '{"id": "t", "label": "t", "outputs": [], "inputs": [{"name": "n", "label": "N", "type": "%s", "required": True}]}' -FINDINGS: dict[str, Finding] = { - "F1": Finding( - "A number parameter given something that is not a number raises a raw Python exception", - 'from labconstrictor_tools import convert\nschema = %s\nconvert.load_inputs(schema, {"n": "abc"}) # ValueError; [1] -> TypeError;' - ' float("inf") -> OverflowError (type "float": 10**400 -> OverflowError)' % (SCHEMA % "integer"), - "ToolError('invalid_parameter', \"'N' must be an integer\") (or a number): the host is told FAILURE code invalid_parameter, " - "as docs/PROTOCOL.md promises for every value-rule violation", - "FAILURE with the Python exception class as code, e.g. [ValueError] invalid literal for int() with base 10: 'abc'", - ), - "F5": Finding( - "A tool that returns the wrong kind of value for an output gets a raw Python exception, not a ToolError('bad_return')", - "a tool returns [[1, 0, 0], [0, 1], [0, 0, 1]] (ragged) or [['a', 0, 0], [0, 1, 0], [0, 0, 1]] as its Affine output; or " - "convert.build_results(schema_with_a_points_output, False, folder); or {} as Affine, None as an image, 5 as Scalars", - "ToolError('bad_return', '... must be ...') as for a matrix of the wrong shape or a missing y/x column, so the author " - "gets the same readable failure for every kind of wrong return", - "FAILURE with the exception class as code: ValueError (inhomogeneous shape / could not convert string to float), " - "TypeError, ...", - ), - "F6": Finding( - "An Affine output with NaN or infinity is delivered as null instead of being refused", - "a tool returns [[1, 0, float('nan')], [0, 1, 0], [0, 0, 1]] as its Affine output", - "ToolError('bad_return', ...) like PointsOut and ShapesOut, and like the author test harness's contract check " - "('affines are finite 3x3')", - "COMPLETE with matrix_yx containing null; a host that applies it moves the image to nowhere", - ), - "F11": Finding( - "A malformed or stale shared-memory image descriptor raises a raw exception", - 'convert.load_inputs(schema, {"image": {"appose_type": "ndarray"}}) -> KeyError; an unknown "shm" name -> FileNotFoundError', - "ToolError('unreadable_image', 'the shared-memory image ... is gone or malformed')", - "FAILURE with code KeyError / FileNotFoundError / TypeError / ValueError", - ), - "F12": Finding( - "A file or folder parameter given a non-text JSON value raises TypeError", - 'convert.load_inputs(schema_with_a_file_parameter, {"f": [1]}) -> TypeError from Path([1])', - "ToolError('invalid_parameter', 'must be a path (text)')", - "FAILURE code TypeError", - ), - "F15": Finding( - "An image output that no host can open is written (or crashes with KeyError) instead of being refused", - "a tool declared -> ImageOut returns np.zeros((0, 3), np.uint8) or np.zeros((2, 2), np.complex64) or " - "np.array([[None, 1]], dtype=object) or np.array([['a', 'b']]) (examples.roundtrip.make_array)", - "ToolError('bad_return' / 'unsupported_dtype', ...): an empty image is refused on the way in (empty_image) and every " - "host fails on a complex or text TIFF, so the tool is told at once", - "empty and complex arrays are written (the empty one as a nonconformant TIFF that the bridge itself refuses to read back); " - "object and text arrays fail with [KeyError] 'O' / 'U'", - ), - "F16": Finding( - "Hints on the wrong kind of parameter, or with an impossible value, are accepted and silently ignored", - "Annotated[str, Min(0)], Annotated[int, Min(5), Max(1)], Annotated[str, Axes('YX')], Annotated[int, PixelSizeOf('image')], " - "Annotated[str, Replace()] (an output marker on an input), two outputs both named with Name('same'): " - "describe_tool accepts every one of them (tests/test_roundtrip_hints.py generates the full list)", - "a clear DeclarationError that names the parameter and what to change: docs/MANIFEST.md asks for 'a clear declaration " - "error for misuse' for every marker; an author who wrote Min on a text parameter or Min(5), Max(1) has made a mistake", - "the hint disappears from the schema (or an empty range / a renamed output is published): the app registers and the " - "mistake is found by a person, if at all. (Duplicate explicit output names become 'same' and 'same2': the " - "'duplicate output names' DeclarationError in introspection._outputs can never be raised)", - ), - "F20": Finding( - "Integer images are checked for float32 exactness with an out-of-range cast: uint64 2**64-1 is stored as 2**64 on ARM", - "from labconstrictor_tools.convert import portable_dtype; import numpy as np\n" - "portable_dtype(np.array([[2**64 - 1, 1]], np.uint64)) # x86: ToolError unsupported_dtype; arm64 (Apple silicon): a float32 array holding 2**64", - "ToolError('unsupported_dtype', ...) on every machine: 2**64-1 is not exactly representable as float32 (docs/PROTOCOL.md: integers never change in value)", - "convert.portable_dtype tests exactness with np.array_equal(widened.astype(array.dtype), array); casting the float32 2**64 back to uint64 is " - "undefined: x86 gives 0 (not equal, refused), ARM saturates to 2**64-1 (equal, accepted) so the value is silently changed by 1. " - "Seen on GitHub's macOS runners (arm64); not reproducible on x86 Linux", - ("posix", "nt"), - (0,), - ("arm64", "aarch64"), - ), -} +FINDINGS: dict[str, Finding] = {} KNOWN: dict[str, str] = {} @@ -109,101 +40,6 @@ def _add(finding: str, *keys: str) -> None: KNOWN[key] = finding -_add( - "F1", - "worker:echo_float/dict", - "worker:echo_float/empty-text", - "worker:echo_float/huge-int", - "worker:echo_float/list", - "worker:echo_float/word", - "worker:echo_int/decimal-text", - "worker:echo_int/dict", - "worker:echo_int/empty-text", - "worker:echo_int/inf", - "worker:echo_int/list", - "worker:echo_int/minus-inf", - "worker:echo_int/nan", - "worker:echo_int/word", - "property:only_toolerror_integer", - "property:only_toolerror_float", - "property:only_toolerror_bounded", -) -_add( - "F5", - "property:only_toolerror_result_affine", - "property:only_toolerror_result_image", - "property:only_toolerror_result_points", - "property:only_toolerror_result_table", - "property:only_toolerror_result_values", - "failure:wrong_type_image", - "failure:wrong_type_table", - "failure:wrong_type_scalars", - "failure:wrong_type_points", - "failure:wrong_count_one_array", - "failure:wrong_count_tuple_for_one", - "worker:echo_affine/invalid/ragged", - "worker:echo_affine/invalid/text-cell", -) -_add( - "F6", - "worker:echo_affine/invalid/inf-cell", - "worker:echo_affine/invalid/nan-cell", -) -_add( - "F11", - "property:only_toolerror_image_shared_memory", -) -_add( - "F15", - "worker:make_array/empty", - "worker:make_array/complex", - "worker:make_array/object", - "worker:make_array/text", -) -_add( - "F20", - "worker:echo_image/uint64/beyond-float32", - "cli:echo_image/uint64/beyond-float32", - "terminal:echo_image/uint64/beyond-float32", - "snippet:echo_image/uint64/beyond-float32", - "notebook:echo_image/uint64/beyond-float32", -) -_add( - "F12", - "property:only_toolerror_file_value", - "property:only_toolerror_folder_value", -) -_add( - "F16", - "hint:ApplyTo-on-the-input-image", - "hint:ApplyTo-on-the-input-str", - "hint:Axes-on-file", - "hint:Axes-on-int", - "hint:Axes-on-str", - "hint:Axes-on-table", - "hint:Max-on-bool", - "hint:Max-on-file", - "hint:Max-on-image", - "hint:Max-on-literal3", - "hint:Max-on-str", - "hint:Min-above-Max", - "hint:Min-is-not-a-number", - "hint:Min-on-bool", - "hint:Min-on-file", - "hint:Min-on-image", - "hint:Min-on-literal3", - "hint:Min-on-str", - "hint:Name-on-the-input-image", - "hint:Name-on-the-input-str", - "hint:PixelSizeOf-on-bool", - "hint:PixelSizeOf-on-int", - "hint:PixelSizeOf-on-str", - "hint:Replace-on-the-input-image", - "hint:Replace-on-the-input-str", - "hint:duplicate-output-names", -) - - def is_known(key: str) -> bool: """True when `key` is a listed failure on this platform (the runners skip it).""" finding = KNOWN.get(key) diff --git a/tests/test_error_codes.py b/tests/test_error_codes.py new file mode 100644 index 0000000..f44f602 --- /dev/null +++ b/tests/test_error_codes.py @@ -0,0 +1,372 @@ +"""Error codes and wording of the value rules, as docs/PROTOCOL.md promises them: a wrong value, a wrong return, a stale +shared-memory block, an image no host can open and a misused hint are each a ToolError (or DeclarationError) with a readable +message, never a raw Python exception. The exact texts are the contract the Groovy hosts and the Napari plugin mirror. +""" + +import unittest +import warnings +from multiprocessing import shared_memory +from typing import Annotated + +import _paths # noqa: F401 (must come first) +import numpy as np + +from labconstrictor_tools import convert, tool +from labconstrictor_tools import types as T +from labconstrictor_tools.introspection import DeclarationError, describe_tool +from labconstrictor_tools.types import ToolError + + +def param(kind, **extra): + return {"name": "n", "label": "N", "type": kind, "required": True, **extra} + + +def load(kind, value, **extra): + return convert._load_one(param(kind, **extra), value) + + +def refused(call, *args, **kwargs): + with warnings.catch_warnings(): + warnings.simplefilter("error") # an out-of-range cast would warn on x86 and be silent on ARM + try: + call(*args, **kwargs) + except ToolError as error: + return error + raise AssertionError("accepted") + + +def result(kind, value, **extra): + import tempfile + + with tempfile.TemporaryDirectory() as folder: + return convert.build_results({"outputs": [{"name": "o", "type": kind, **extra}]}, value, folder) + + +class NumbersMustBeNumbers(unittest.TestCase): + def test_integer_given_something_else_is_invalid_parameter_naming_what_arrived(self): + for value, arrived in (("abc", "text"), ([1], "a list"), ({}, "an object"), (None, "nothing (None)")): + with self.subTest(value=value): + error = refused(load, "integer", value) + self.assertEqual(error.code, "invalid_parameter") + self.assertEqual(error.message, "'N' must be an integer, got %s" % arrived) + + def test_integer_that_is_not_whole_or_not_finite(self): + for value in (1.5, float("inf"), float("nan"), True): + with self.subTest(value=value): + self.assertEqual(refused(load, "integer", value).code, "invalid_parameter") + self.assertEqual(load("integer", 3.0), 3) + + def test_float_given_something_else_or_beyond_the_float_range(self): + self.assertEqual(refused(load, "float", "1.5").message, "'N' must be a number, got text") + self.assertEqual(refused(load, "float", 10**400).message, "'N' must be a finite number") + self.assertEqual(load("float", 2), 2.0) + + +class PathsMustBeText(unittest.TestCase): + def test_file_folder_and_image_given_a_non_text_value(self): + for kind in ("file", "folder", "image"): + for value, arrived in (([1], "a list"), (5, "an integer"), ({}, "an object")): + with self.subTest(kind=kind, value=value): + error = refused(load, kind, value) + self.assertEqual(error.code, "invalid_parameter") + self.assertEqual( + error.message, + "'N' must be a path (text), got %s: the host must send the file or folder path as text" + % arrived, + ) + + +class SharedMemoryDescriptors(unittest.TestCase): + def descriptor(self, **changes): + base = { + "appose_type": "ndarray", + "shm": {"name": "lc_no_such_block"}, + "shape": [2, 2], + "dtype": "uint8", + } + return {**base, **changes} + + def test_a_block_that_is_gone_or_a_descriptor_with_missing_or_wrong_fields(self): + cases = { + "gone": self.descriptor(), + "no-shm": {"appose_type": "ndarray", "shape": [1], "dtype": "uint8"}, + "shm-not-an-object": self.descriptor(shm=5), + "name-not-text": self.descriptor(shm={"name": 5}), + "no-dtype": {k: v for k, v in self.descriptor().items() if k != "dtype"}, + "no-shape": {k: v for k, v in self.descriptor().items() if k != "shape"}, + "bad-dtype": self.descriptor(dtype="nonsense"), + "object-dtype": self.descriptor(dtype="object"), + "shape-not-a-list": self.descriptor(shape=5), + } + for label, descriptor in cases.items(): + with self.subTest(label): + error = refused(load, "image", descriptor) + self.assertEqual(error.code, "unreadable_image") + self.assertIn("the shared-memory image for 'N' is gone or malformed", error.message) + self.assertIn("the host must keep the block alive until the task ends", error.message) + + def test_a_shape_larger_than_the_block_or_negative(self): + block = shared_memory.SharedMemory(create=True, size=16) + try: + too_big = block.size + ( + 1 << 16 + ) # beyond the real block even where blocks are rounded up to whole pages + for shape in ([too_big, 1], [-1, 4]): + with self.subTest(shape=shape): + descriptor = self.descriptor(shm={"name": block.name}, shape=shape) + self.assertEqual(refused(load, "image", descriptor).code, "unreadable_image") + good = load("image", self.descriptor(shm={"name": block.name}, shape=[4, 4])) + self.assertEqual(good.shape, (4, 4)) + finally: + block.close() + block.unlink() + + +class WrongReturns(unittest.TestCase): + def test_each_kind_of_wrong_value_names_the_output_and_what_was_expected(self): + cases = [ + ("image", None, "the image output 'o' must be an array of numbers, got nothing (None)"), + ("image", "text", "the image output 'o' must be an array of numbers, got text"), + ("labels", {}, "the labels output 'o' must be an array of numbers, got an object"), + ( + "table", + 5, + "the table output 'o' must be a DataFrame, a dict of lists or a list of dicts, got an integer", + ), + ("values", [1, 2], "the values output 'o' must be a dict of JSON-able values, got a list"), + ("file", 5, "the file output 'o' must be a path (pathlib.Path or text), got an integer"), + ("message", 5, "the message output 'o' must be text, got an integer"), + ( + "points", + False, + "the points output 'o' must be a DataFrame, a dict of lists or a list of dicts, got true/false", + ), + ] + for kind, value, message in cases: + with self.subTest(kind=kind, value=value): + error = refused(result, kind, value) + self.assertEqual((error.code, error.message), ("bad_return", message)) + + def test_table_like_mistakes_are_bad_return_whatever_the_pandas_version(self): + mistakes = [ + [{}, None], + [1, 2], + [{"a": 1}, [1]], + ["a", {}], + {"a": {"b": 1}}, + {"a": [1, 2], "b": [1]}, + {"a": 1}, + [[1, 2], [3]], + [None], + "abc", + {1, 2}, + np.array(5), + np.zeros((2, 2)), + iter([1]), + {"a": np.zeros((2, 2))}, + ] + for kind in ("table", "points"): + for value in mistakes: + with self.subTest(kind=kind, value=repr(value)[:30]): + self.assertEqual(refused(result, kind, value).code, "bad_return") + self.assertIn("different lengths", refused(result, "table", {"a": [1, 2], "b": [1]}).message) + + def test_a_nul_or_a_surrogate_in_a_csv_result_is_refused_the_same_on_every_python(self): + import pandas as pd + + for kind, base in (("table", {}), ("points", {"y": [1.0], "x": [2.0]})): + cases = { + "nul-in-a-cell": ({**base, "t": ["a\x00b"]}, "a NUL character in a cell of column 't'"), + "nul-in-a-name": ({**base, "t\x00": ["a"]}, "a NUL character in the name of column 't\\x00'"), + "nul-alone": ({**base, "": ["\x00"]}, "a NUL character in a cell of column ''"), + "surrogate-in-a-cell": ({**base, "t": ["\ud800"]}, "cannot be written as a UTF-8 CSV"), + "surrogate-in-a-name": ({**base, "\ud800": ["a"]}, "cannot be written as a UTF-8 CSV"), + } + for label, (value, fragment) in cases.items(): + with self.subTest(kind=kind, case=label): + error = refused(result, kind, value) + self.assertEqual(error.code, "bad_return") + self.assertIn(fragment, error.message) + twin = pd.DataFrame([["a", "b\x00"]], columns=["d", "d"]) + twin_error = refused(result, kind, twin) if kind == "table" else None + if twin_error: + self.assertIn("a NUL character in a cell of column 'd'", twin_error.message) + ok = {**base, "t": ["x\r\ny", 'q"uote', "c,omma"]} if kind == "table" else {**base} + self.assertEqual(result(kind, pd.DataFrame(ok if kind == "table" else base))[0]["type"], kind) + + def test_affine_that_is_ragged_text_or_not_a_matrix(self): + for value in ([[1, 0, 0], [0, 1], [0, 0, 1]], [["a", 0, 0], [0, 1, 0], [0, 0, 1]], {}): + with self.subTest(value=value): + error = refused(result, "affine", value) + self.assertEqual(error.code, "bad_return") + self.assertIn("the affine output 'o' must be a 3x3 matrix of numbers", error.message) + + def test_affine_with_nan_or_infinity_is_refused(self): + for bad in (float("nan"), float("inf"), float("-inf")): + with self.subTest(bad=bad): + error = refused(result, "affine", [[1, 0, bad], [0, 1, 0], [0, 0, 1]]) + self.assertEqual(error.code, "bad_return") + self.assertIn("the affine output 'o' has a value that is not a finite number", error.message) + + def test_two_outputs_given_one_array_or_a_scalar_is_bad_return_not_a_crash(self): + schema = {"outputs": [{"name": "a", "type": "image"}, {"name": "b", "type": "table"}]} + for returned in (np.zeros((2, 2), np.uint8), 5, "ab"): + with self.subTest(returned=type(returned).__name__): + error = refused(convert.build_results, schema, returned, ".") + self.assertEqual(error.code, "bad_return") + self.assertIn("declared 2 outputs: return a tuple with one value per output", error.message) + + +class ImagesNoHostCanOpen(unittest.TestCase): + def test_empty_arrays_are_bad_return(self): + for shape in ((0, 3), (0,), (2, 0, 2)): + with self.subTest(shape=shape): + error = refused(convert.portable_dtype, np.zeros(shape, np.uint8), "image") + self.assertEqual(error.code, "bad_return") + self.assertIn("image is empty (shape %s)" % (shape,), error.message) + + def test_complex_object_and_text_arrays_are_unsupported_dtype(self): + arrays = { + "complex": np.zeros((2, 2), np.complex64), + "object": np.array([[None, 1]], dtype=object), + "text": np.array([["a", "b"]]), + "bytes": np.array([[b"a"]]), + } + for label, array in arrays.items(): + with self.subTest(label): + error = refused(convert.portable_dtype, array, "image") + self.assertEqual(error.code, "unsupported_dtype") + self.assertIn("hosts can only open numbers; return a numeric array", error.message) + + +class Float32Exactness(unittest.TestCase): + """Never cast a float back to an integer type outside its range: x86 gives 0 and ARM saturates, so only a comparison that + stays inside the range is exact on every CPU. numpy warns about the bad cast on x86, which these tests turn into failures. + """ + + def test_uint64_beyond_float32_is_refused_with_unsupported_dtype_on_every_cpu(self): + for value in (2**64 - 1, 2**64 - 2**39, 2**63 + 1): + with self.subTest(value=value): + error = refused(convert.portable_dtype, np.array([[value, 1]], np.uint64)) + self.assertEqual(error.code, "unsupported_dtype") + + def test_int64_extremes_are_refused_or_exact_without_a_bad_cast(self): + info = np.iinfo(np.int64) + self.assertEqual( + refused(convert.portable_dtype, np.array([info.max], np.int64)).code, "unsupported_dtype" + ) + with warnings.catch_warnings(): + warnings.simplefilter("error") + out = convert.portable_dtype(np.array([info.min, 2**40], np.int64)) # powers of two are exact + self.assertEqual((out.dtype, out.tolist()), (np.float32, [float(info.min), float(2**40)])) + + def test_uint64_that_float32_holds_exactly_is_kept(self): + with warnings.catch_warnings(): + warnings.simplefilter("error") + out = convert.portable_dtype(np.array([2**63, 2**40 * 3], np.uint64)) + self.assertEqual((out.dtype, out.tolist()), (np.float32, [float(2**63), float(2**40 * 3)])) + + +def describe(fn): + """The real @tool decorator, then the real describe_tool: the DeclarationError an author sees at registration.""" + return describe_tool(tool("t")(fn).__lc_tool__) + + +def declaration_error(fn): + try: + describe(fn) + except DeclarationError as error: + return str(error) + raise AssertionError("accepted") + + +class HintMisuse(unittest.TestCase): + """The wording of each refusal: it names the parameter (or tool), the hint and the fix.""" + + def test_bounds_on_text_and_an_empty_range(self): + def min_on_text(p: Annotated[str, T.Min(0)]) -> T.Scalars: ... + + def min_above_max(p: Annotated[int, T.Min(5), T.Max(1)]) -> T.Scalars: ... + + def bound_not_a_number(p: Annotated[int, T.Min("a")]) -> T.Scalars: ... + + self.assertEqual( + declaration_error(min_on_text), + "parameter 'p': Min applies to an int or float parameter; this one is of type string: remove it", + ) + self.assertEqual( + declaration_error(min_above_max), + "parameter 'p': Min(5) is above Max(1), so no value is allowed: swap them or widen the range", + ) + self.assertEqual( + declaration_error(bound_not_a_number), "parameter 'p': Min('a') must be a finite number" + ) + + def test_axes_and_pixel_size_on_the_wrong_type(self): + def axes_on_text(p: Annotated[str, T.Axes("YX")]) -> T.Scalars: ... + + def axes_on_table(p: Annotated[T.Table, T.Axes("YX")]) -> T.Scalars: ... + + def pixel_size_on_int(image: T.Image, p: Annotated[int, T.PixelSizeOf("image")]) -> T.Scalars: ... + + def axes_on_a_table_output(image: T.Image) -> Annotated[T.TableOut, T.Axes("YX")]: ... + + self.assertEqual( + declaration_error(axes_on_text), + "parameter 'p': Axes describes the dimensions of an Image or Labels input and this parameter is not one: remove it", + ) + self.assertEqual( + declaration_error(axes_on_table), + "parameter 'p': Axes describes the dimensions of an Image or Labels input and this parameter is not one: remove it", + ) + self.assertEqual( + declaration_error(pixel_size_on_int), + "parameter 'p': PixelSizeOf applies to a float parameter (the calibration in um/px); this one is of type integer: remove it", + ) + self.assertEqual( + declaration_error(axes_on_a_table_output), + "tool 'axes_on_a_table_output' output 0: Axes describes an image or labels output; this one is of type table: remove it", + ) + + def test_output_markers_on_an_input(self): + def name_on_input(p: Annotated[str, T.Name("x")]) -> T.Scalars: ... + + def replace_on_input(p: Annotated[T.Image, T.Replace()]) -> T.Scalars: ... + + def apply_to_on_input(p: Annotated[str, T.ApplyTo("p")]) -> T.Scalars: ... + + for fn, marker in ( + (name_on_input, "Name"), + (replace_on_input, "Replace"), + (apply_to_on_input, "ApplyTo"), + ): + with self.subTest(marker): + self.assertEqual( + declaration_error(fn), + "parameter 'p': %s describes a result, not an input: put it on the return annotation or remove it" + % marker, + ) + + def test_duplicate_output_names(self): + def same( + image: T.Image, + ) -> tuple[Annotated[T.ImageOut, T.Name("same")], Annotated[T.TableOut, T.Name("same")]]: ... + + def collides( + image: T.Image, + ) -> tuple[T.ImageOut, T.ImageOut, Annotated[T.TableOut, T.Name("image2")]]: ... + + self.assertEqual( + declaration_error(same), + "tool 'same': two outputs are named 'same': give each output its own Name(...)", + ) + self.assertIn("duplicate output names", declaration_error(collides)) + + def test_unnamed_outputs_of_one_type_still_get_a_numeric_suffix(self): + def two(image: T.Image) -> tuple[T.ImageOut, T.ImageOut]: ... + + self.assertEqual([o["name"] for o in describe(two)["outputs"]], ["image", "image2"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_roundtrip_known_failures.py b/tests/test_roundtrip_known_failures.py index e410712..a04be17 100644 --- a/tests/test_roundtrip_known_failures.py +++ b/tests/test_roundtrip_known_failures.py @@ -170,10 +170,11 @@ def test_windows_process_entries_still_fail(self): def test_the_skip_is_exactly_the_list(self): self.assertFalse(is_known("worker:echo_int/ok/0")) - self.assertTrue(is_known("worker:echo_int/word")) - self.assertFalse( - is_known("cli:echo_table/missing-file") - ) # F4 is fixed: its ids are no longer skipped + # the sentinel is any listed key (the list shrinks as findings are fixed, so no id is named here) + listed_here = [key for key in KNOWN if is_known(key)] + self.assertEqual(bool(listed_here), bool(KNOWN)) + for key in listed_here: + self.assertTrue(is_known(key)) def tearDownModule(): diff --git a/tests/test_roundtrip_properties.py b/tests/test_roundtrip_properties.py index 6cb9754..c041e11 100644 --- a/tests/test_roundtrip_properties.py +++ b/tests/test_roundtrip_properties.py @@ -24,7 +24,7 @@ import roundtrip_harness as harness import roundtrip_profiles # noqa: F401 (registers and loads the hypothesis profile) import tifffile -from hypothesis import given, settings +from hypothesis import example, given, settings from hypothesis import strategies as st from hypothesis.extra import numpy as hnp from roundtrip_known_failures import KNOWN, is_known @@ -171,6 +171,37 @@ def only_toolerror_result_affine(value): only_toolerror_results("affine", value) +# shapes that crashed some pandas versions (found by hypothesis on Python 3.10): pinned so every Python and every run sees them +TABLE_LIKE_MISTAKES = ( + [{}, None], + [1, 2], + [{"a": 1}, [1]], + ["a", {}], + {"a": {"b": 1}}, + {"a": [1, 2], "b": [1]}, + {"a": 1}, + [[1, 2], [3]], + [None], + "abc", + {1, 2}, + {"": [None, []]}, + {"a": np.zeros((2, 2))}, + {"": ["\x00"]}, + {"a\x00b": [1]}, + {"a": ["\ud800"]}, + {"\ud800": [1]}, + {"a": ["x\r\ny", 'q"uote', "c,omma", "\n"]}, + {'a\r\n",': [1]}, +) + + +def with_table_examples(test): + for mistake in TABLE_LIKE_MISTAKES: + test = example(mistake)(test) + return test + + +@with_table_examples @given(JSON_VALUES) def only_toolerror_result_points(value): only_toolerror_results("points", value) @@ -181,6 +212,7 @@ def only_toolerror_result_shapes(value): only_toolerror_results("shapes", value) +@with_table_examples @given(JSON_VALUES) def only_toolerror_result_table(value): only_toolerror_results("table", value) diff --git a/tests/test_sabotage.py b/tests/test_sabotage.py index 6b17823..56520a7 100644 --- a/tests/test_sabotage.py +++ b/tests/test_sabotage.py @@ -82,15 +82,15 @@ class Break: Break( "convert: a boolean is accepted as an integer", "convert.py", - "if isinstance(value, bool) or int(value) != value:", - "if int(value) != value:", + "return isinstance(value, (int, float)) and not isinstance(value, bool)", + "return isinstance(value, (int, float))", matrix("echo_int/bool"), ), Break( "convert: a fraction is accepted as an integer", "convert.py", - "if isinstance(value, bool) or int(value) != value:", - "if isinstance(value, bool):", + "if isinstance(value, float) and not (math.isfinite(value) and value.is_integer()):", + "if False:", matrix("echo_int/(fraction|tiny)"), ), Break( @@ -131,7 +131,7 @@ class Break: Break( "convert: a File arrives as text", "convert.py", - ' return Path(value)\n if kind == "folder":', + ' return _path_text(param["label"], value)\n if kind == "folder":', ' return str(value)\n if kind == "folder":', matrix("echo_file/name/plain"), ), @@ -187,8 +187,8 @@ class Break: Break( "convert: a message is not stripped", "convert.py", - "text = str(value).strip()", - "text = str(value)", + "text = value.strip()", + "text = value", matrix("echo_message/surrounded"), ), Break(