diff --git a/CHANGELOG.md b/CHANGELOG.md index 377ee13..1761c5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ All notable user-facing changes to DerZug are documented here. - Dimension choosers no longer derive the default from the alphabetically sorted dim list; when a patch has no `time` dimension the default is now the first patch dimension, matching `resolve_patch_dim`. - The Fourier widget's inverse-transform dimension fallback now prefers a `ft_*` axis (matching `FourierTask.run`) instead of the forward-transform default, which preferred `time`. - Aggregate's phase-weighted stack no longer fails headlessly when no stack dimension is set; it defaults to `distance` (else the first patch dimension), the same default the widget applies, and the widget's transform-dimension chooser now excludes that effective stack dimension. +- The Coords widget now builds and validates its task exclusively through the node layer, so the task that runs on the canvas and the task exported into a saved workflow are always identical. Two behavior fixes come with it: headless callers filling only the `set_coords` draft fields (`set_coords_dim`/`start`/`stop`/`step`) now get a real coordinate update instead of a silent no-op, and data-flipping a non-dimension coordinate now reports through the "Invalid flip selection" banner instead of a generic operation failure. +- The Spool widget's preview/output pipeline now runs the same node-layer select and chunk stages as a headless workflow. A chunk value that parses to `None` (e.g. the text `None`) now disables chunking headlessly, matching the canvas chunk controls. ### Changed (breaking) diff --git a/src/derzug/nodes/coords.py b/src/derzug/nodes/coords.py index 5578bc8..e5fb912 100644 --- a/src/derzug/nodes/coords.py +++ b/src/derzug/nodes/coords.py @@ -27,6 +27,65 @@ def normalize_mapping_rows(rows: object) -> list[list[str]]: return output +class CoordsValidationError(ValueError): + """Raised when persisted coords parameters do not fit the incoming patch. + + ``kind`` and ``label`` let the widget route the failure to the matching + error banner without re-implementing the validation itself. + """ + + def __init__(self, kind: str, detail: str, label: str = "") -> None: + super().__init__(detail) + self.kind = kind + self.label = label + self.detail = detail + + +def resolve_set_coord(patch, dim: str, start: str, stop: str, step: str): + """Return the replacement coordinate built from sparse set-coords text. + + Raises ``CoordsValidationError`` when the dimension is missing, a value + does not parse, no value is given, or the coordinate cannot be built. + Shared by ``CoordsTask.run`` and the widget's draft validation so both + accept and reject exactly the same inputs. + """ + if dim not in patch.dims: + raise CoordsValidationError( + "set_coords", f"'{dim}' is not an available dimension" + ) + coord = patch.coords.get_coord(dim) + parsed: dict[str, object] = {} + for label, raw in (("start", start), ("stop", stop), ("step", step)): + text = str(raw).strip() + if not text: + continue + try: + value = parse_coord_text_value(text, getattr(coord, label), None) + except Exception as exc: + raise CoordsValidationError( + "set_coords", f"could not parse {label}: {exc}" + ) from exc + parsed[label] = value + if not parsed: + raise CoordsValidationError( + "set_coords", "at least one of start, stop, and step is required" + ) + if set(parsed) == {"start"} or set(parsed) == {"stop"}: + parsed["step"] = coord.step + elif set(parsed) == {"step"}: + parsed["start"] = coord.start + kwargs = { + "shape": patch.shape[patch.dims.index(dim)], + "units": coord.units, + "dtype": coord.dtype, + **parsed, + } + try: + return get_coord(**kwargs) + except Exception as exc: + raise CoordsValidationError("set_coords", str(exc)) from exc + + class CoordsTask(Task): """Portable coordinate-operation task for the Coords widget.""" @@ -60,10 +119,14 @@ def _normalize_rows(rows: tuple[tuple[str, str], ...]) -> list[tuple[str, str]]: def _validate_mapping( rows: tuple[tuple[str, str], ...], *, + label: str, valid_left: tuple[str, ...], valid_right: tuple[str, ...] | None, reject_duplicate_right: bool, ) -> dict[str, str]: + def _fail(detail: str): + raise CoordsValidationError("mapping", detail, label=label) + mapping: dict[str, str] = {} valid_left_set = set(valid_left) valid_right_set = None if valid_right is None else set(valid_right) @@ -72,69 +135,38 @@ def _validate_mapping( if not left and not right: continue if not left or not right: - raise ValueError("both columns must be filled") + _fail("both columns must be filled") if left not in valid_left_set: - raise ValueError(f"'{left}' is not available") + _fail(f"'{left}' is not available") if valid_right_set is not None and right not in valid_right_set: - raise ValueError(f"'{right}' is not available") + _fail(f"'{right}' is not available") if left in mapping: - raise ValueError(f"duplicate source '{left}'") + _fail(f"duplicate source '{left}'") if reject_duplicate_right and right in used_right: - raise ValueError(f"duplicate target '{right}'") + _fail(f"duplicate target '{right}'") mapping[left] = right used_right.add(right) if not mapping: - raise ValueError("at least one mapping is required") + _fail("at least one mapping is required") return mapping @staticmethod def _validate_selection( - selected: tuple[str, ...], valid: tuple[str, ...] + selected: tuple[str, ...], valid: tuple[str, ...], *, label: str ) -> list[str]: valid_set = set(valid) out = [str(item) for item in selected] invalid = [name for name in out if name not in valid_set] if invalid: - raise ValueError(", ".join(invalid)) + raise CoordsValidationError("selection", ", ".join(invalid), label=label) return out - @staticmethod - def _parse_set_coord_value(text: str, sample: object) -> object: - return parse_coord_text_value(str(text), sample, None) - - def _resolved_coord(self, patch): - dim = self.set_coords_applied_dim - if dim not in patch.dims: - raise ValueError(f"'{dim}' is not an available dimension") - coord = patch.coords.get_coord(dim) - parsed: dict[str, object] = {} - for label, raw in ( - ("start", self.set_coords_applied_start), - ("stop", self.set_coords_applied_stop), - ("step", self.set_coords_applied_step), - ): - text = str(raw).strip() - if not text: - continue - parsed[label] = self._parse_set_coord_value(text, getattr(coord, label)) - if not parsed: - raise ValueError("at least one of start, stop, and step is required") - if set(parsed) == {"start"}: - parsed["step"] = coord.step - elif set(parsed) == {"stop"}: - parsed["step"] = coord.step - elif set(parsed) == {"step"}: - parsed["start"] = coord.start - kwargs = { - "shape": patch.shape[patch.dims.index(dim)], - "units": coord.units, - "dtype": coord.dtype, - **parsed, - } - return get_coord(**kwargs) + def _validated_call(self, patch): + """Validate parameters against ``patch`` and return the deferred call. - def run(self, patch): - """Apply the selected coordinate operation to one patch.""" + Validation is eager so ``preflight`` can reuse it; the returned + zero-argument callable performs the actual patch operation. + """ operation = str(self.operation or "rename_coords") available_dims = tuple(patch.dims) available_coords = tuple(patch.coords.coord_map) @@ -145,81 +177,105 @@ def run(self, patch): if operation == "rename_coords": mapping = self._validate_mapping( self.rename_rows, + label="rename", valid_left=available_coords, valid_right=None, reject_duplicate_right=True, ) - return patch.rename_coords(**mapping) + return lambda: patch.rename_coords(**mapping) if operation == "drop_coords": selected = self._validate_selection( - self.drop_coords_selected, - non_dim_coords, + self.drop_coords_selected, non_dim_coords, label="drop" ) - return patch if not selected else patch.drop_coords(*selected) + return lambda: patch if not selected else patch.drop_coords(*selected) if operation == "sort_coords": selected = self._validate_selection( - self.sort_coords_selected, - available_coords, + self.sort_coords_selected, available_coords, label="sort" ) - return ( + return lambda: ( patch if not selected else patch.sort_coords(*selected, reverse=bool(self.sort_reverse)) ) if operation == "snap_coords": selected = self._validate_selection( - self.snap_coords_selected, - available_coords, + self.snap_coords_selected, available_coords, label="snap" ) - return ( + return lambda: ( patch if not selected else patch.snap_coords(*selected, reverse=bool(self.snap_reverse)) ) if operation == "set_coords": if not self.set_coords_applied_dim: - return patch - return patch.update_coords( - **{self.set_coords_applied_dim: self._resolved_coord(patch)} + return lambda: patch + coord = resolve_set_coord( + patch, + self.set_coords_applied_dim, + self.set_coords_applied_start, + self.set_coords_applied_stop, + self.set_coords_applied_step, ) + return lambda: patch.update_coords(**{self.set_coords_applied_dim: coord}) if operation == "set_dims": mapping = self._validate_mapping( self.set_dims_rows, + label="set_dims", valid_left=available_dims, valid_right=available_coords, reject_duplicate_right=True, ) - return patch.set_dims(**mapping) + return lambda: patch.set_dims(**mapping) if operation == "flip": selected = self._validate_selection( - self.flip_dims_selected, - available_coords, + self.flip_dims_selected, available_coords, label="flip" ) if not selected or (not self.flip_data and not self.flip_coords): - return patch + return lambda: patch dim_names = tuple(name for name in selected if name in available_dims) if self.flip_data and len(dim_names) != len(selected): invalid = [name for name in selected if name not in available_dims] - raise ValueError( + raise CoordsValidationError( + "selection", "data flip requires dimension coordinates; " - f"non-dim coords selected: {', '.join(invalid)}" + f"non-dim coords selected: {', '.join(invalid)}", + label="flip", ) - out = patch - if self.flip_data and dim_names: - out = out.flip(*dim_names, flip_coords=False) - if self.flip_coords: - out = out.update(coords=out.coords.flip(*tuple(selected))) - return out + + def _flip(): + out = patch + if self.flip_data and dim_names: + out = out.flip(*dim_names, flip_coords=False) + if self.flip_coords: + out = out.update(coords=out.coords.flip(*tuple(selected))) + return out + + return _flip if operation == "transpose": order = list(self.transpose_order) - dims = list(available_dims) if not order: - return patch - if sorted(order) != sorted(dims): - raise ValueError("dimension order does not match the input patch") - return patch.transpose(*order) + return lambda: patch + if sorted(order) != sorted(available_dims): + raise CoordsValidationError( + "selection", + "dimension order does not match the input patch", + label="transpose", + ) + return lambda: patch.transpose(*order) raise ValueError(f"Unknown coords operation '{operation}'") + def preflight(self, patch) -> None: + """Validate persisted parameters against one patch without running. + + Raises ``CoordsValidationError`` on the first problem, letting the + widget surface the same failures its banners used to compute itself. + """ + self._validated_call(patch) + + def run(self, patch): + """Apply the selected coordinate operation to one patch.""" + return self._validated_call(patch)() + class CoordsParams(BaseModel): """Parameters for the Coords widget (all affect the output patch).""" @@ -255,9 +311,36 @@ class CoordsParams(BaseModel): transpose_order: list = Field(default_factory=list) +def _applied_set_coords_fields(params: CoordsParams) -> tuple[str, str, str, str]: + """Return the effective applied set-coords fields, promoting drafts. + + On the canvas the draft fields are the source of truth: every patch + arrival re-derives the ``*_applied_*`` mirror from them. Headlessly the + same precedence applies — a non-empty draft wins, and the applied fields + only carry a hand-authored update when no draft is present. + """ + draft = ( + str(params.set_coords_dim or ""), + str(params.set_coords_start or ""), + str(params.set_coords_stop or ""), + str(params.set_coords_step or ""), + ) + if draft[0] and any(value.strip() for value in draft[1:]): + return draft + return ( + str(params.set_coords_applied_dim or ""), + str(params.set_coords_applied_start or ""), + str(params.set_coords_applied_stop or ""), + str(params.set_coords_applied_step or ""), + ) + + def coords_task_from_params(params: CoordsParams | None = None) -> CoordsTask: """Build the configured coordinate-operation task.""" params = CoordsParams() if params is None else params + applied_dim, applied_start, applied_stop, applied_step = _applied_set_coords_fields( + params + ) return CoordsTask( operation=params.operation, rename_rows=tuple( @@ -268,10 +351,10 @@ def coords_task_from_params(params: CoordsParams | None = None) -> CoordsTask: (str(left), str(right)) for left, right in normalize_mapping_rows(params.set_dims_rows) ), - set_coords_applied_dim=str(params.set_coords_applied_dim or ""), - set_coords_applied_start=str(params.set_coords_applied_start or ""), - set_coords_applied_stop=str(params.set_coords_applied_stop or ""), - set_coords_applied_step=str(params.set_coords_applied_step or ""), + set_coords_applied_dim=applied_dim, + set_coords_applied_start=applied_start, + set_coords_applied_stop=applied_stop, + set_coords_applied_step=applied_step, drop_coords_selected=tuple(params.drop_coords_selected or ()), sort_coords_selected=tuple(params.sort_coords_selected or ()), sort_reverse=bool(params.sort_reverse), diff --git a/src/derzug/nodes/spool.py b/src/derzug/nodes/spool.py index ccd553f..c9c3d8b 100644 --- a/src/derzug/nodes/spool.py +++ b/src/derzug/nodes/spool.py @@ -222,6 +222,21 @@ def load_spool_from_settings( return result +def parse_spool_scalar(text: str) -> Any | None: + """Parse one chunk/select text value; blank text or ``None`` yields None. + + Ellipsis (``...``) and other Python literals parse via ``literal_eval``; + non-literal text passes through as the raw string. + """ + stripped = str(text).strip() + if not stripped: + return None + try: + return ast.literal_eval(stripped) + except Exception: + return stripped + + def apply_select_rows( spool: dc.BaseSpool, select_filters: tuple[dict[str, str], ...], @@ -230,14 +245,8 @@ def apply_select_rows( kwargs = {} for filter_data in select_filters: key = str(filter_data.get("key", "")).strip() - raw_value = str(filter_data.get("raw", "")).strip() - if not key or not raw_value: - continue - try: - value = ast.literal_eval(raw_value) - except Exception: - value = raw_value - if value is None: + value = parse_spool_scalar(filter_data.get("raw", "")) + if not key or value is None: continue kwargs[key] = value if not kwargs: @@ -257,27 +266,21 @@ def apply_chunk_settings( chunk_tolerance: float, chunk_conflict: str, ) -> dc.BaseSpool: - """Apply persisted chunk settings to a spool.""" + """Apply persisted chunk settings to a spool. + + A chunk value that parses to None (blank or the text ``None``) disables + chunking, matching the widget's chunk controls. + """ if not bool(chunk_enabled): return spool dim = chunk_dim.strip() - raw_value = chunk_value.strip() - if not dim or not raw_value: + value = parse_spool_scalar(chunk_value) + if not dim or value is None: return spool - try: - value = ast.literal_eval(raw_value) - except Exception: - value = raw_value - overlap = None - if chunk_overlap.strip(): - try: - overlap = ast.literal_eval(chunk_overlap.strip()) - except Exception: - overlap = chunk_overlap.strip() return spool.chunk( **{ dim: value, - "overlap": overlap, + "overlap": parse_spool_scalar(chunk_overlap), "keep_partial": bool(chunk_keep_partial), "snap_coords": bool(chunk_snap_coords), "tolerance": float(chunk_tolerance), @@ -286,6 +289,28 @@ def apply_chunk_settings( ) +def apply_spool_transforms( + spool: dc.BaseSpool, task: SpoolTask | SpoolTransformTask +) -> dc.BaseSpool: + """Apply one task's persisted select and chunk settings to a spool. + + The single definition of the select -> chunk order, shared by both node + tasks and the widget's snapshot executor. + """ + spool = apply_select_rows(spool, task.select_filters) + return apply_chunk_settings( + spool, + chunk_enabled=task.chunk_enabled, + chunk_dim=task.chunk_dim, + chunk_value=task.chunk_value, + chunk_overlap=task.chunk_overlap, + chunk_keep_partial=task.chunk_keep_partial, + chunk_snap_coords=task.chunk_snap_coords, + chunk_tolerance=task.chunk_tolerance, + chunk_conflict=task.chunk_conflict, + ) + + class SpoolParams(BaseModel): """Parameters for the Spool source widget (source + chunk + select config).""" @@ -350,18 +375,7 @@ def run(self): ) if selected_row is None: selected_row = self.selected_source_row - spool = apply_select_rows(source, self.select_filters) - spool = apply_chunk_settings( - spool, - chunk_enabled=self.chunk_enabled, - chunk_dim=self.chunk_dim, - chunk_value=self.chunk_value, - chunk_overlap=self.chunk_overlap, - chunk_keep_partial=self.chunk_keep_partial, - chunk_snap_coords=self.chunk_snap_coords, - chunk_tolerance=self.chunk_tolerance, - chunk_conflict=self.chunk_conflict, - ) + spool = apply_spool_transforms(source, self) if selected_row is not None: spool = spool_rows_to_output(spool, {int(selected_row)}) patch = extract_single_patch(spool) if self.unpack_single_patch else None @@ -391,18 +405,7 @@ class SpoolTransformTask(Task): def run(self, spool): """Apply select/chunk settings to an input spool.""" - spool = apply_select_rows(spool, self.select_filters) - spool = apply_chunk_settings( - spool, - chunk_enabled=self.chunk_enabled, - chunk_dim=self.chunk_dim, - chunk_value=self.chunk_value, - chunk_overlap=self.chunk_overlap, - chunk_keep_partial=self.chunk_keep_partial, - chunk_snap_coords=self.chunk_snap_coords, - chunk_tolerance=self.chunk_tolerance, - chunk_conflict=self.chunk_conflict, - ) + spool = apply_spool_transforms(spool, self) if self.selected_source_row is not None: spool = spool_rows_to_output(spool, {int(self.selected_source_row)}) patch = extract_single_patch(spool) if self.unpack_single_patch else None diff --git a/src/derzug/widgets/coords.py b/src/derzug/widgets/coords.py index 6dfcdc6..4d6ab91 100644 --- a/src/derzug/widgets/coords.py +++ b/src/derzug/widgets/coords.py @@ -24,14 +24,17 @@ QVBoxLayout, QWidget, ) -from dascore.core.coords import get_coord from Orange.widgets import gui from Orange.widgets.utils.signals import Input, Output from Orange.widgets.widget import Msg from derzug.core.zugwidget import WidgetExecutionRequest, ZugWidget -from derzug.nodes.coords import NODE_SPEC, CoordsTask -from derzug.utils.parsing import parse_coord_text_value +from derzug.nodes.coords import ( + NODE_SPEC, + CoordsTask, + CoordsValidationError, + resolve_set_coord, +) from derzug.workflow import Task @@ -836,249 +839,37 @@ def _run(self) -> dc.Patch | None: ) def _validated_task(self) -> CoordsTask | None: - """Return the current validated coordinate task, or None on invalid state.""" + """Return the current coordinate task, or None when preflight fails. + + The node factory is the only task constructor; the node's ``preflight`` + runs the same validation ``run`` would and its structured error is + mapped onto the widget's banners. + """ operation = self._coerce_operation() self._set_current_operation_ui(operation) - if operation == "rename_coords": - mapping = self._validated_mapping( - self.rename_rows, - label="rename", - valid_left=self._available_coords, - valid_right=None, - reject_duplicate_right=True, - ) - if mapping is None: - return None - rename_rows = tuple(mapping.items()) - return CoordsTask(operation=operation, rename_rows=rename_rows) - if operation == "drop_coords": - selected = self._validated_selection( - self.drop_coords_selected, - self._available_non_dim_coords, - label="drop", - ) - if selected is None: - return None - return CoordsTask( - operation=operation, - drop_coords_selected=tuple(selected), - ) - if operation == "sort_coords": - selected = self._validated_selection( - self.sort_coords_selected, - self._available_coords, - label="sort", - ) - if selected is None: - return None - return CoordsTask( - operation=operation, - sort_coords_selected=tuple(selected), - sort_reverse=bool(self.sort_reverse), - ) - if operation == "snap_coords": - selected = self._validated_selection( - self.snap_coords_selected, - self._available_coords, - label="snap", - ) - if selected is None: - return None - return CoordsTask( - operation=operation, - snap_coords_selected=tuple(selected), - snap_reverse=bool(self.snap_reverse), - ) - if operation == "set_coords": - if not self.set_coords_applied_dim: - return CoordsTask(operation=operation) - coord = self._validated_set_coords_coord() - if coord is None: + task = self._task_snapshot() + if self._patch is not None: + try: + task.preflight(self._patch) + except CoordsValidationError as exc: + self._show_coords_validation_error(exc) return None - return CoordsTask( - operation=operation, - set_coords_applied_dim=str(self.set_coords_applied_dim or ""), - set_coords_applied_start=str(self.set_coords_applied_start or ""), - set_coords_applied_stop=str(self.set_coords_applied_stop or ""), - set_coords_applied_step=str(self.set_coords_applied_step or ""), - ) - if operation == "set_dims": - mapping = self._validated_mapping( - self.set_dims_rows, - label="set_dims", - valid_left=self._available_dims, - valid_right=self._available_coords, - reject_duplicate_right=True, - ) - if mapping is None: - return None - return CoordsTask( - operation=operation, - set_dims_rows=tuple(mapping.items()), - ) - if operation == "flip": - selected = self._validated_selection( - self.flip_dims_selected, - self._available_coords, - label="flip", - ) - if selected is None: - return None - return CoordsTask( - operation=operation, - flip_dims_selected=tuple(selected), - flip_data=bool(self.flip_data), - flip_coords=bool(self.flip_coords), - ) - order = self._validated_transpose_order() - if order is None: - return None - return CoordsTask( - operation=operation, - transpose_order=tuple(order), - ) + return task + + def _show_coords_validation_error(self, exc: CoordsValidationError) -> None: + """Route one structured node validation failure to its banner.""" + if exc.kind == "mapping": + self._show_error_message("invalid_mapping", exc.label, exc.detail) + elif exc.kind == "selection": + self._show_error_message("invalid_selection", exc.label, exc.detail) + else: + self._show_error_message("invalid_set_coords", exc.detail) def _task_snapshot(self) -> CoordsTask: """Return the stored coordinate-operation state without patch validation.""" self._coerce_operation() return NODE_SPEC.build_task(self.get_params()) - def _validated_mapping( - self, - rows: object, - *, - label: str, - valid_left: tuple[str, ...], - valid_right: tuple[str, ...] | None, - reject_duplicate_right: bool, - ) -> dict[str, str] | None: - """Validate mapping-table rows and return kwargs for DASCore.""" - mapping: dict[str, str] = {} - used_right: set[str] = set() - valid_left_set = set(valid_left) - valid_right_set = None if valid_right is None else set(valid_right) - - for left, right in self._normalize_rows(rows): - left = left.strip() - right = right.strip() - if not left and not right: - continue - if not left or not right: - self._show_error_message( - "invalid_mapping", - label, - "both columns must be filled", - ) - return None - if left not in valid_left_set: - self._show_error_message( - "invalid_mapping", - label, - f"'{left}' is not available", - ) - return None - if valid_right_set is not None and right not in valid_right_set: - self._show_error_message( - "invalid_mapping", - label, - f"'{right}' is not available", - ) - return None - if left in mapping: - self._show_error_message( - "invalid_mapping", - label, - f"duplicate source '{left}'", - ) - return None - if reject_duplicate_right and right in used_right: - self._show_error_message( - "invalid_mapping", - label, - f"duplicate target '{right}'", - ) - return None - mapping[left] = right - used_right.add(right) - - if not mapping: - self._show_error_message( - "invalid_mapping", - label, - "at least one mapping is required", - ) - return None - return mapping - - def _validated_selection( - self, - selected: object, - valid: tuple[str, ...], - *, - label: str, - ) -> list[str] | None: - """Validate serialized coord-name selections.""" - selected_names = [str(item) for item in (selected or [])] - valid_set = set(valid) - invalid = [name for name in selected_names if name not in valid_set] - if invalid: - self._show_error_message( - "invalid_selection", - label, - ", ".join(invalid), - ) - return None - return selected_names - - def _validated_transpose_order(self) -> list[str] | None: - """Validate the transpose dimension order against the input patch.""" - order = list(self.transpose_order) - dims = list(self._available_dims) - if not dims: - return [] - if sorted(order) != sorted(dims): - self._show_error_message( - "invalid_selection", - "transpose", - "dimension order does not match the input patch", - ) - return None - self.transpose_order = order - return order - - def _validated_set_coords_coord(self): - """Return the replacement coordinate resolved from sparse applied state.""" - dim = self.set_coords_applied_dim - if dim not in self._available_dims: - self._show_error_message( - "invalid_set_coords", - f"'{dim}' is not an available dimension", - ) - return None - - coord = self._patch.coords.get_coord(dim) - axis_len = self._patch.shape[self._available_dims.index(dim)] - sparse_values = self._parse_set_coords_values( - self.set_coords_applied_start, - self.set_coords_applied_stop, - self.set_coords_applied_step, - coord, - ) - if sparse_values is None: - return None - - kwargs = { - "shape": axis_len, - "units": coord.units, - "dtype": coord.dtype, - **self._completed_set_coords_kwargs(sparse_values, coord), - } - try: - return get_coord(**kwargs) - except Exception as exc: - self._show_error_message("invalid_set_coords", str(exc)) - return None - def _validated_set_coords_applied_state( self, ) -> tuple[str, str, str, str] | None: @@ -1087,21 +878,16 @@ def _validated_set_coords_applied_state( if dim not in self._available_dims: self._show_error_message("invalid_set_coords", "select a valid dimension") return None - - coord = self._patch.coords.get_coord(dim) - sparse_values = self._parse_set_coords_values( - self.set_coords_start, - self.set_coords_stop, - self.set_coords_step, - coord, - ) - if sparse_values is None: - return None - if not sparse_values: - self._show_error_message( - "invalid_set_coords", - "at least one of start, stop, and step is required", + try: + resolve_set_coord( + self._patch, + dim, + self.set_coords_start, + self.set_coords_stop, + self.set_coords_step, ) + except CoordsValidationError as exc: + self._show_error_message("invalid_set_coords", exc.detail) return None return ( dim, @@ -1110,61 +896,6 @@ def _validated_set_coords_applied_state( self.set_coords_step.strip(), ) - def _parse_set_coords_values( - self, - start_text: str, - stop_text: str, - step_text: str, - coord, - ) -> dict[str, object] | None: - """Parse sparse set-coords text values for one dimension.""" - parsed: dict[str, object] = {} - raw_values = { - "start": str(start_text).strip(), - "stop": str(stop_text).strip(), - "step": str(step_text).strip(), - } - for label, raw in raw_values.items(): - if not raw: - continue - sample = getattr(coord, label) - value = self._parse_set_coords_value(raw, sample, label) - if value is None: - return None - parsed[label] = value - return parsed - - @staticmethod - def _completed_set_coords_kwargs( - sparse_values: dict[str, object], - coord, - ) -> dict[str, object]: - """Fill single-field set-coords updates from the current coordinate.""" - names = set(sparse_values) - if names == {"start"}: - return {"start": sparse_values["start"], "step": coord.step} - if names == {"stop"}: - return {"stop": sparse_values["stop"], "step": coord.step} - if names == {"step"}: - return {"start": coord.start, "step": sparse_values["step"]} - return dict(sparse_values) - - def _parse_set_coords_value( - self, - text: str, - sample: object, - label: str, - ) -> object | None: - """Parse one optional set-coords draft/applied value.""" - try: - return parse_coord_text_value(str(text), sample, None) - except Exception as exc: - self._show_error_message( - "invalid_set_coords", - f"could not parse {label}: {exc}", - ) - return None - def _on_result(self, result: dc.Patch | None) -> None: """Send the output patch and refresh the textual preview.""" self._last_result = result diff --git a/src/derzug/widgets/spool.py b/src/derzug/widgets/spool.py index efd5d0e..57a8894 100644 --- a/src/derzug/widgets/spool.py +++ b/src/derzug/widgets/spool.py @@ -4,9 +4,7 @@ from __future__ import annotations -import ast import datetime -from collections.abc import Callable from dataclasses import dataclass from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any @@ -43,8 +41,7 @@ SpoolParams, SpoolTask, SpoolTransformTask, - apply_chunk_settings, - apply_select_rows, + apply_spool_transforms, contents_identity_token, load_spool_from_settings, ordered_contents_df, @@ -58,7 +55,6 @@ from derzug.utils.display import format_display from derzug.utils.dynamic_rows import DynamicRowManager from derzug.utils.example_parameters import ( - build_example_call_kwargs, filter_example_overrides, get_example_parameter_specs, ) @@ -259,18 +255,19 @@ def _emit_task( visible_row_count: int | None, ) -> tuple[dc.BaseSpool | None, dc.Patch | None]: """Read selected patch data off the main thread and return (spool, patch).""" - if not selected_source_rows: + if selected_source_rows: + output_spool = spool_rows_to_output(display_spool, selected_source_rows) + output_row_count = len(selected_source_rows) + else: output_spool = display_spool - if unpack_single and visible_row_count == 1: - output_patch = extract_single_patch(output_spool) - else: - output_patch = None + output_row_count = visible_row_count + # Unpack exactly one output row, same rule as extract_single_patch in the + # node tasks; the row count is known here so the extract call is skipped + # for multi-row outputs. + if unpack_single and output_row_count == 1: + output_patch = extract_single_patch(output_spool) else: - output_spool = spool_rows_to_output(display_spool, selected_source_rows) - if unpack_single and len(selected_source_rows) == 1: - output_patch = extract_single_patch(output_spool) - else: - output_patch = None + output_patch = None return output_spool, output_patch @@ -397,24 +394,8 @@ def _execute_spool_snapshot(snapshot: _SpoolExecutionSnapshot) -> _SpoolExecutio except Exception as exc: raise _SettingsSourceLoadError(str(exc)) from exc try: - display_spool = apply_select_rows(source_spool, task.select_filters) - display_spool = apply_chunk_settings( - display_spool, - chunk_enabled=task.chunk_enabled, - chunk_dim=task.chunk_dim, - chunk_value=task.chunk_value, - chunk_overlap=task.chunk_overlap, - chunk_keep_partial=task.chunk_keep_partial, - chunk_snap_coords=task.chunk_snap_coords, - chunk_tolerance=task.chunk_tolerance, - chunk_conflict=task.chunk_conflict, - ) - visible_row_count = _spool_row_count(display_spool) - output_spool, output_patch = _emit_task( - display_spool, - snapshot.selected_source_rows, - task.unpack_single_patch, - visible_row_count, + display_spool, output_spool, output_patch = _transform_and_emit( + source_spool, task, snapshot.selected_source_rows ) except Exception as exc: raise _SettingsTransformError( @@ -435,24 +416,8 @@ def _execute_spool_snapshot(snapshot: _SpoolExecutionSnapshot) -> _SpoolExecutio None, None, None, None, display_generation=snapshot.display_generation ) assert isinstance(task, SpoolTransformTask) - display_spool = apply_select_rows(source_spool, task.select_filters) - display_spool = apply_chunk_settings( - display_spool, - chunk_enabled=task.chunk_enabled, - chunk_dim=task.chunk_dim, - chunk_value=task.chunk_value, - chunk_overlap=task.chunk_overlap, - chunk_keep_partial=task.chunk_keep_partial, - chunk_snap_coords=task.chunk_snap_coords, - chunk_tolerance=task.chunk_tolerance, - chunk_conflict=task.chunk_conflict, - ) - visible_row_count = _spool_row_count(display_spool) - output_spool, output_patch = _emit_task( - display_spool, - snapshot.selected_source_rows, - task.unpack_single_patch, - visible_row_count, + display_spool, output_spool, output_patch = _transform_and_emit( + source_spool, task, snapshot.selected_source_rows ) return _SpoolExecutionResult( source_spool=source_spool, @@ -464,6 +429,22 @@ def _execute_spool_snapshot(snapshot: _SpoolExecutionSnapshot) -> _SpoolExecutio ) +def _transform_and_emit( + source_spool: dc.BaseSpool, + task: SpoolTask | SpoolTransformTask, + selected_source_rows: frozenset[int], +) -> tuple[dc.BaseSpool, dc.BaseSpool | None, dc.Patch | None]: + """Run the node's select/chunk stages, then emit the selected outputs.""" + display_spool = apply_spool_transforms(source_spool, task) + output_spool, output_patch = _emit_task( + display_spool, + selected_source_rows, + task.unpack_single_patch, + _spool_row_count(display_spool), + ) + return display_spool, output_spool, output_patch + + class Spool(ZugWidget): """Orange widget for loading DASCore example spools.""" @@ -894,39 +875,6 @@ def _on_update_clicked(self) -> None: self._set_source_spool(updated) self.run() - def _snapshot_loader(self) -> tuple[str | None, Callable | None]: - """Capture current source state and return (source_name, pure_callable). - - All widget-state reads happen here on the main thread. The returned - callable captures only immutable data so it is safe to run in a worker. - """ - if self.file_input: - path = self.file_input - return path, lambda: dc.spool(path) - - if self.raw_input: - raw = self.raw_input - return raw, lambda: dc.spool(raw) - - if not self._examples: - self.Warning.no_examples() - return None, None - example_name = self.spool_input - if not example_name: - return None, None - fn = self._selected_example_callable(example_name) - kwargs = build_example_call_kwargs( - fn, self.example_parameters_for(example_name) - ) - - def _load() -> dc.BaseSpool: - result = fn(**kwargs) - if isinstance(result, dc.Patch): - return dc.spool([result]) - return result - - return example_name, _load - def _supports_async_execution(self) -> bool: """Load and transform spool data off-thread by default.""" return True @@ -1334,24 +1282,6 @@ def _clear_other_inputs(self, active: str) -> None: self.raw_edit.clear() self.raw_edit.blockSignals(False) - def _load_from_example(self) -> dc.BaseSpool: - """Load spool from the selected example key.""" - if not self._examples: - self.Warning.no_examples() - raise ValueError("No examples available") - example_name = self._selected_example_name() - if example_name is None: - raise ValueError("No example selected") - fn = self._selected_example_callable(example_name) - kwargs = build_example_call_kwargs( - fn, - self.example_parameters_for(example_name), - ) - result = fn(**kwargs) - if isinstance(result, dc.Patch): - return dc.spool([result]) - return result - def _selected_example_name(self) -> str | None: """Return the currently selected example name, if any.""" return self.spool_input @@ -1627,18 +1557,6 @@ def _on_chunk_dim_changed(self, index: int) -> None: self.chunk_dim = dim self._on_chunk_param_changed() - def _parse_chunk_scalar(self, text: str) -> Any: - """Parse text for chunk kwargs, supporting literals and ellipsis.""" - t = text.strip() - if not t: - return None - if t == "...": - return ... - try: - return ast.literal_eval(t) - except Exception: - return t - def _on_chunk_param_changed(self, *_args) -> None: """Apply chunk with selected parameters and emit chunked spool.""" if self._source_spool is None: @@ -1686,8 +1604,7 @@ def _recompute_display_spool(self) -> None: try: # Narrow the spool before any downstream patch materialization so # chunking and row extraction work on the smallest candidate set. - display = self._apply_select_transform(source) - display = self._apply_chunk_transform(display) + display = apply_spool_transforms(source, self._current_transform_task()) except Exception as exc: self._show_exception("general", exc) return @@ -1699,38 +1616,6 @@ def _refresh_ui(self) -> None: """Refresh the visible table and transform controls.""" self._render_spool(self._display_spool) - def _apply_chunk_transform(self, spool: dc.BaseSpool) -> dc.BaseSpool: - """Return the source spool or a chunked derivative based on current controls.""" - if not bool(self.chunk_enabled): - return spool - dim = (self.chunk_dim or "").strip() - value = self._parse_chunk_scalar(self.chunk_value) - if not dim or value is None: - return spool - - overlap = self._parse_chunk_scalar(self.chunk_overlap) - kwargs = { - dim: value, - "overlap": overlap, - "keep_partial": bool(self.chunk_keep_partial), - "snap_coords": bool(self.chunk_snap_coords), - "tolerance": float(self.chunk_tolerance), - "conflict": self.chunk_conflict, - } - return spool.chunk(**kwargs) - - def _apply_select_transform(self, spool: dc.BaseSpool) -> dc.BaseSpool: - """Return the input spool or a selection-filtered derivative.""" - kwargs = {} - for filter_data in self._iter_active_select_filters(): - value = self._parse_chunk_scalar(filter_data["raw"]) - if value is None: - continue - kwargs[filter_data["key"]] = value - if not kwargs: - return spool - return spool.select(**kwargs) - @staticmethod def _blank_select_filter() -> dict[str, str]: """Return one empty select-filter entry.""" @@ -1819,15 +1704,6 @@ def _serialize_select_row(self, row: dict[str, QWidget]) -> dict[str, str]: "raw": row["edit"].text().strip(), } - def _iter_active_select_filters(self) -> list[dict[str, str]]: - """Return non-empty select filters from the current widget state.""" - self._sync_select_filters_from_ui() - return [ - item - for item in self.select_filters - if item["key"].strip() and item["raw"].strip() - ] - def _on_add_select_row_clicked(self) -> None: """Append a blank select row and re-run selection.""" self._select_row_manager.add_blank_row() diff --git a/tests/test_nodes/test_coords.py b/tests/test_nodes/test_coords.py new file mode 100644 index 0000000..93938d9 --- /dev/null +++ b/tests/test_nodes/test_coords.py @@ -0,0 +1,118 @@ +"""Tests for the Qt-free Coords node.""" + +from __future__ import annotations + +import dascore as dc +import numpy as np +import pytest +from derzug.nodes.coords import ( + CoordsParams, + CoordsValidationError, + coords_task_from_params, +) + + +@pytest.fixture +def patch() -> dc.Patch: + """Return a small example patch.""" + return dc.get_example_patch("random_das") + + +class TestSetCoordsDraftPromotion: + """Headless draft fields must run like the widget's applied promotion.""" + + def test_draft_fields_alone_apply(self, patch): + """A headless author filling only the draft fields gets a real update.""" + params = CoordsParams( + operation="set_coords", set_coords_dim="distance", set_coords_start="10" + ) + + out = coords_task_from_params(params).run(patch) + + assert float(out.get_array("distance")[0]) == pytest.approx(10.0) + + def test_drafts_win_over_applied_fields(self, patch): + """Drafts beat the applied mirror, as canvas rehydration would.""" + params = CoordsParams( + operation="set_coords", + set_coords_dim="distance", + set_coords_start="99", + set_coords_applied_dim="distance", + set_coords_applied_start="10", + ) + + out = coords_task_from_params(params).run(patch) + + assert float(out.get_array("distance")[0]) == pytest.approx(99.0) + + def test_applied_fields_used_when_no_draft(self, patch): + """Hand-authored applied fields still run when drafts are empty.""" + params = CoordsParams( + operation="set_coords", + set_coords_applied_dim="distance", + set_coords_applied_start="10", + ) + + out = coords_task_from_params(params).run(patch) + + assert float(out.get_array("distance")[0]) == pytest.approx(10.0) + + def test_empty_draft_values_stay_a_noop(self, patch): + """A draft dim with no start/stop/step still passes through.""" + params = CoordsParams(operation="set_coords", set_coords_dim="distance") + + out = coords_task_from_params(params).run(patch) + + assert np.array_equal(out.get_array("distance"), patch.get_array("distance")) + + +class TestPreflight: + """preflight must raise the same structured errors run would.""" + + def test_valid_params_pass(self, patch): + """A valid rename preflights without raising.""" + params = CoordsParams( + operation="rename_coords", rename_rows=[["distance", "offset"]] + ) + coords_task_from_params(params).preflight(patch) + + def test_invalid_mapping_raises_structured_error(self, patch): + """A rename from a missing coord raises with banner routing info.""" + params = CoordsParams( + operation="rename_coords", rename_rows=[["missing", "offset"]] + ) + task = coords_task_from_params(params) + + with pytest.raises(CoordsValidationError) as info: + task.preflight(patch) + + assert info.value.kind == "mapping" + assert info.value.label == "rename" + assert "'missing' is not available" in info.value.detail + + def test_invalid_transpose_raises_structured_error(self, patch): + """A transpose order not matching the patch raises a selection error.""" + params = CoordsParams(operation="transpose", transpose_order=["time"]) + task = coords_task_from_params(params) + + with pytest.raises(CoordsValidationError) as info: + task.preflight(patch) + + assert info.value.kind == "selection" + assert info.value.label == "transpose" + + def test_data_flip_of_non_dim_coord_raises_structured_error(self, patch): + """Data-flipping a non-dimension coordinate is rejected in preflight.""" + with_coord = patch.update_coords( + quality=("distance", np.arange(patch.shape[patch.dims.index("distance")])) + ) + params = CoordsParams( + operation="flip", flip_dims_selected=["quality"], flip_coords=False + ) + task = coords_task_from_params(params) + + with pytest.raises(CoordsValidationError) as info: + task.preflight(with_coord) + + assert info.value.kind == "selection" + assert "data flip requires dimension coordinates" in info.value.detail diff --git a/tests/test_nodes/test_spool_node.py b/tests/test_nodes/test_spool_node.py index 8eb75b8..b8930e2 100644 --- a/tests/test_nodes/test_spool_node.py +++ b/tests/test_nodes/test_spool_node.py @@ -12,7 +12,9 @@ from derzug.nodes.spool import ( NODE_SPEC, SpoolParams, + apply_chunk_settings, contents_identity_token, + load_spool_from_settings, ordered_contents_df, resolved_select_filters, spool_task_from_params, @@ -83,3 +85,80 @@ def test_row_index_is_used_when_no_token_was_saved(self, directory_spool): selected = ordered_contents_df(result["spool"]) assert len(selected) == 1 assert contents_identity_token(selected, 0) == contents_identity_token(df, 0) + + +class TestLoadSpoolFromSettings: + """Source routing lives in one place for the widget and the node.""" + + def test_file_input_wins_over_other_sources(self, monkeypatch): + """A file path beats raw input and the example selection.""" + calls: list[str] = [] + monkeypatch.setattr(dc, "spool", lambda arg: calls.append(arg) or "loaded") + + out = load_spool_from_settings( + spool_input="plain_example", + example_parameters={}, + file_input=" /data/spool_dir ", + raw_input="raw-source", + ) + + assert out == "loaded" + assert calls == ["/data/spool_dir"] + + def test_raw_input_used_when_no_file(self, monkeypatch): + """Raw input loads when no file path is set.""" + calls: list[str] = [] + monkeypatch.setattr(dc, "spool", lambda arg: calls.append(arg) or "loaded") + + out = load_spool_from_settings( + spool_input=None, + example_parameters={}, + file_input="", + raw_input="raw-source", + ) + + assert out == "loaded" + assert calls == ["raw-source"] + + def test_example_parameter_overrides_reach_the_callable(self, monkeypatch): + """Saved per-example overrides are applied to the example call.""" + captured: dict[str, object] = {} + + def example(sample_rate: int = 150): + captured["sample_rate"] = sample_rate + return dc.get_example_spool("random_das") + + monkeypatch.setattr( + "derzug.nodes.spool.all_examples", lambda ignore=(): {"ex": example} + ) + + load_spool_from_settings( + spool_input="ex", + example_parameters={"ex": {"sample_rate": 220}}, + file_input="", + raw_input="", + ) + + assert captured["sample_rate"] == 220 + + +class TestApplyChunkSettings: + """Chunk-text parsing must behave like the widget's chunk controls.""" + + def test_none_chunk_value_disables_chunking(self): + """The text 'None' no-ops instead of chunking with a None value.""" + spool = dc.get_example_spool("random_das") + + out = apply_chunk_settings( + spool, + chunk_enabled=True, + chunk_dim="time", + chunk_value="None", + chunk_overlap="", + chunk_keep_partial=False, + chunk_snap_coords=True, + chunk_tolerance=1.5, + chunk_conflict="raise", + ) + + assert out is spool diff --git a/tests/test_widgets/test_coords.py b/tests/test_widgets/test_coords.py index 466f8c1..f2103d8 100644 --- a/tests/test_widgets/test_coords.py +++ b/tests/test_widgets/test_coords.py @@ -580,7 +580,10 @@ def test_flip_data_rejects_non_dim_coords(self, coords_widget, monkeypatch, qtbo wait_for_output(qtbot, received) assert received[-1] is None - assert coords_widget.Error.operation_failed.is_shown() + assert coords_widget.Error.invalid_selection.is_shown() + assert "data flip requires dimension coordinates" in ( + coords_widget.Error.invalid_selection.formatted + ) def test_flip_active_summary_describes_selected_dims(self, coords_widget): """Preview text should summarize flip state.""" diff --git a/tests/test_widgets/test_spool.py b/tests/test_widgets/test_spool.py index 08b2e77..4ac8e4b 100644 --- a/tests/test_widgets/test_spool.py +++ b/tests/test_widgets/test_spool.py @@ -312,22 +312,22 @@ def test_run_emits_spool(self, spool_widget, monkeypatch, qtbot): assert len(received) == 1 assert received[0] is not None - def test_load_from_example_uses_saved_parameter_overrides(self, monkeypatch): - """Example-specific overrides should be passed into the selected callable.""" + def test_saved_parameter_overrides_reach_the_output(self, monkeypatch, qtbot): + """Example-specific overrides should shape the emitted spool.""" examples = _make_example_map() monkeypatch.setattr( "derzug.nodes.spool.all_examples", lambda ignore=(): examples ) with widget_context(Spool) as widget: + received = capture_output(widget.Outputs.spool, monkeypatch) widget.spool_input = "configured_example" widget.example_parameters = { "configured_example": {"sample_rate": 220, "duration": 2.0} } + _run_and_wait(widget, qtbot) - spool = widget._load_from_example() - - patch = next(iter(spool)) + patch = next(iter(received[-1])) time = patch.get_array("time") assert len(time) == 440 assert time[1] - time[0] == pytest.approx(1 / 220) @@ -733,24 +733,6 @@ def test_file_or_dir_dialog_accepts_file(self, tmp_path): # Necessary to convert to a Path object for OS-independent equivalence assert Path(dialog.chosen_path()) == file_path - def test_run_uses_file_loader_when_file_input_set(self, spool_widget): - """_snapshot_loader() routes to the file path when file_input is set.""" - spool_widget.file_input = "/tmp/fake" - spool_widget.raw_input = "" - spool_widget.spool_input = None - source_name, loader_fn = spool_widget._snapshot_loader() - assert source_name == "/tmp/fake" - assert callable(loader_fn) - - def test_run_uses_raw_loader_when_raw_input_set(self, spool_widget): - """_snapshot_loader() routes to the raw path when raw_input is set.""" - spool_widget.raw_input = "raw://fake" - spool_widget.file_input = "" - spool_widget.spool_input = None - source_name, loader_fn = spool_widget._snapshot_loader() - assert source_name == "raw://fake" - assert callable(loader_fn) - def test_run_with_no_selection_emits_none(self, spool_widget, monkeypatch): """run() with no selected example sends None downstream.""" received = capture_output(spool_widget.Outputs.spool, monkeypatch) @@ -2237,7 +2219,8 @@ def test_append_uses_source_spool_not_chunked_display( derived = dc.spool([_patch_with_tag("derived-only")]) spool_widget._current_spool = dc.spool([first]) monkeypatch.setattr( - spool_widget, "_apply_chunk_transform", lambda spool: derived + "derzug.widgets.spool.apply_spool_transforms", + lambda spool, task: derived, ) monkeypatch.setattr(spool_widget, "_render_spool", lambda spool: None) monkeypatch.setattr(spool_widget, "run", lambda: None) @@ -2259,19 +2242,19 @@ def test_recompute_display_applies_select_before_chunk( chunked = dc.spool([_patch_with_tag("chunked")]) calls: list[tuple[str, object]] = [] - def _select(spool): + def _select(spool, select_filters): calls.append(("select", spool)) assert spool is base return selected - def _chunk(spool): + def _chunk(spool, **kwargs): calls.append(("chunk", spool)) assert spool is selected return chunked spool_widget._source_spool = base - monkeypatch.setattr(spool_widget, "_apply_select_transform", _select) - monkeypatch.setattr(spool_widget, "_apply_chunk_transform", _chunk) + monkeypatch.setattr("derzug.nodes.spool.apply_select_rows", _select) + monkeypatch.setattr("derzug.nodes.spool.apply_chunk_settings", _chunk) monkeypatch.setattr(spool_widget, "_render_spool", lambda spool: None) spool_widget._recompute_display_spool()