Fix write validation against empty transformations#1118
Merged
LucaMarconato merged 3 commits intomainfrom May 7, 2026
Merged
Conversation
…tions
After remove_transformation(element, remove_all=True) the transformations
dict is set to {} rather than None, bypassing the is-None guard in all three
IO writers. Changed the check to `not transformations` so both None and empty
dicts are caught, and added a parametrized regression test covering images,
multiscale images, labels, multiscale labels, shapes, and points.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of guarding in each IO writer, call get_model(element) (which
already dispatches to the right schema and runs validate()) at the start
of _write_element() for all non-table spatial elements.
Also fix the is-None guards in all three validate() methods to use
`not transformations` / `not data.attrs.get(key)` so that an empty
dict {} is caught in addition to None.
The IO-level guards added in the previous commit are removed since they
are now superseded by the model-level check; assert statements are kept
to narrow the type for mypy.
The regression test is updated to reflect the correct production scenario:
element is already inside a SpatialData object when its transformations are
removed in-place, so the error fires during write() not at construction.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1118 +/- ##
==========================================
+ Coverage 91.97% 92.03% +0.05%
==========================================
Files 51 51
Lines 7750 7757 +7
==========================================
+ Hits 7128 7139 +11
+ Misses 622 618 -4
🚀 New features to boost your workflow:
|
Instead of guarding in each IO writer, call validate_element(element) (a new public helper in spatialdata.models that delegates to get_model) at the start of _write_element() for all non-table spatial elements. Validation changes in models.py: - RasterSchema._check_transforms_present: two explicit checks — one for None (key absent) and one for empty dict, with separate messages - ShapesModel.validate / PointsModel.validate: same split into two checks - asserts in IO files are kept solely for mypy type-narrowing, each annotated with a comment explaining that validate_element() guarantees the invariant at runtime New public API: - spatialdata.models.validate_element(e) raises ValueError if the element fails schema validation; documented in docs/api/models_utils.md Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The parsers add transformations to empty elements, but after parsing, if a user calls
remove_transformation(element, remove_all=True), the transformations dict is set to {} rather than None, bypassing the is-None guard in all three IO writers.The fix is general: we now call the validators before writing, and we check the presence of transformations in the validator.