Skip to content

fix(labels): defer float-dtype rejection to spatialdata's model validation - #758

Merged
timtreis merged 1 commit into
mainfrom
fix/spatialdata08-label-dtype
Aug 7, 2026
Merged

fix(labels): defer float-dtype rejection to spatialdata's model validation#758
timtreis merged 1 commit into
mainfrom
fix/spatialdata08-label-dtype

Conversation

@timtreis

Copy link
Copy Markdown
Member

Problem

main is red on 4 label-related tests, all caused by spatialdata 0.8:

  • test_render_labels_rejects_float_dtype[float16/32/64]
  • test_utils.py::TestMeasureObs::test_float_dtype_labels_supported

They fail at test setup, with:

ValueError: Labels must have an integer dtype, found float32. Cast the data, e.g. `.astype(np.uint16)`.

Root cause

#606 (filed against spatialdata 0.5, where Labels2DModel.parse still accepted float32) added a float-dtype guard inside _render_labels. spatialdata 0.8 moved that validation to the model boundary — parse(), SpatialData(...), and sd[k] = ... all now reject non-integer label rasters with a clear, actionable message (verified: the only way to obtain a float-labels element is a raw .data mutation, which is unsupported).

Consequences:

Changes

  • Remove the redundant float-dtype guard in _render_labels; defer to spatialdata's boundary validation (interop over duplication).
  • Droptest_render_labels_rejects_float_dtype — it asserts an unreachable render-time scenario.
  • Rewrite the measure_obs float test to exercise _stream_label_centroid_stats directly (where the integer-valued-float → int cast actually lives), asserting a float raster yields exactly what its integer counterpart yields. The cast itself is kept as cheap correct-by-construction robustness.

Not in scope

The 5th main failure — Shapes_…random_shuffling (visual) — is a separate, deterministic upstream draw-order drift (colors still map correctly; only overlap z-order changed). Tracked for its own baseline-refresh PR, not folded in here.

Testing

  • Full non-visual test_utils.py (140) and test_render_labels.py pass locally.
  • The two obsolete test IDs are removed/renamed; the new helper-level test covers the retained cast.

…ation
spatialdata 0.8 rejects non-integer label rasters at the model boundary
(parse / SpatialData construction / __setitem__) with a clear, actionable
message. This makes _render_labels' own float-dtype guard (added in #606,
when spatialdata 0.5 still accepted float labels) redundant dead code: a
validly built element can no longer reach the renderer with a float dtype.
- Remove the redundant render-time guard; trust the upstream boundary.
- Drop test_render_labels_rejects_float_dtype (asserts an unreachable
render-time scenario; upstream now raises earlier at parse).
- Rewrite the measure_obs float test to exercise _stream_label_centroid_stats
directly (where the integer-valued-float -> int cast lives), since a float
labels element can no longer be placed in a SpatialData.
The helper's float->int cast is kept as cheap, correct-by-construction
robustness for any integer-valued float raster passed to it directly.
@timtreis
timtreis merged commit fddb0d7 into mainAug 7, 2026
4 of 8 checks passed
timtreis added a commit that referenced this pull request Aug 7, 2026
#758 removed the render-time float-dtype guard on the assumption that
spatialdata always rejects float labels at the model boundary. That only
holds on spatialdata >=0.8; the library still supports spatialdata >=0.3,
where float labels parse fine and would again crash deep in skimage with a
cryptic TypeError (#606). Restore the guard so the clear error holds across
the whole supported range.
- render.py: re-add the float-dtype ValueError guard.
- test_render_labels.py: re-add test_render_labels_rejects_float_dtype in a
version-agnostic form (assert an "integer dtype" error is raised at
whichever layer enforces it: spatialdata on >=0.8, our guard on <0.8).
- test_render_shapes.py: sort queried shapes before render so the overlap
draw order (and the baseline) is stable across spatialdata's query-order
change in 0.8; regenerate the baseline accordingly.
The measure_obs float coverage stays as the version-independent
_stream_label_centroid_stats unit test added in #758.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@timtreis