Skip to content

Improve affine decomposition - #1186

Open
LucaMarconato wants to merge 1 commit into
mainfrom
improve/affine-decomposition
Open

Improve affine decomposition#1186
LucaMarconato wants to merge 1 commit into
mainfrom
improve/affine-decomposition

Conversation

@LucaMarconato

@LucaMarconato LucaMarconato commented Aug 20, 2026

Copy link
Copy Markdown
Member

Claude generated PR description, manually verified

Port the RQ-based decomposition algorithm from Spatial-Innovation-Team/transfo
to replace the old 2D-only, theta-based rotation logic in
_decompose_transformation. The new algorithm is dimension-generic, warns on
ill-conditioned linear parts, and encodes the reflection as a single
axis flip instead of an arbitrary diagonal matrix.

Drop the spatialdata-specific restrictions that no longer apply: the z axis
and the c channel are now decomposed like any other axis, and axes are
allowed to come out in a different order between input and output (checked
by axis set rather than exact tuple order).

Split _decompose_transformation into _decompose_transformation_simple and
_decompose_transformation_full, each returning a fixed-size tuple of the
component transformations instead of a Sequence, so callers get a
concretely-typed tuple (2 vs. 5 components) rather than a Union.

Reorganize the tests into TestSimpleDecomposition/TestFullDecomposition
classes, consolidating round-trip coverage into a single
DECOMPOSE_TRANSFORMATION_CASES table (with descriptive ids) shared by both
classes, and keeping only the checks that can't be expressed as a
matrix-in/reconstructed-matrix-out case (warnings, component types/arity,
algorithmic invariants).

@LucaMarconato
LucaMarconato changed the base branch from main to fix/bounding-box-query-axes-dependent-results August 20, 2026 13:07
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.72727% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.40%. Comparing base (11039c7) to head (2f10ecd).

Files with missing lines Patch % Lines
src/spatialdata/transformations/transformations.py 97.72% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1186      +/-   ##
==========================================
- Coverage   92.41%   92.40%   -0.02%     
==========================================
  Files          52       52              
  Lines        7874     7869       -5     
==========================================
- Hits         7277     7271       -6     
- Misses        597      598       +1     
Files with missing lines Coverage Δ
src/spatialdata/transformations/transformations.py 90.82% <97.72%> (-0.26%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LucaMarconato

Copy link
Copy Markdown
Member Author

@Tomaz-Vieira can you please review?

Also, the GitHub diff view is not the most useful for this PR since the code is now split into functions. I suggest to use a standard diff view, so that most of the code is easily shown as untouched.

@LucaMarconato
LucaMarconato changed the base branch from fix/bounding-box-query-axes-dependent-results to main August 20, 2026 13:27
ported from transfo: supporting z and c; improved order of returned
transformations
also: split into simple/full; changed return type to tuples; supporting
permutation of input/output axes for the transformation
@LucaMarconato
LucaMarconato force-pushed the improve/affine-decomposition branch from ed6769d to 2f10ecd Compare August 20, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants