Skip to content

[1/3] Bundle TilingQCParams; add resolve_labels_array helper#1188

Merged
timtreis merged 7 commits into
scverse:mainfrom
timtreis:feature/tiling-qc-params
May 29, 2026
Merged

[1/3] Bundle TilingQCParams; add resolve_labels_array helper#1188
timtreis merged 7 commits into
scverse:mainfrom
timtreis:feature/tiling-qc-params

Conversation

@timtreis

@timtreis timtreis commented May 27, 2026

Copy link
Copy Markdown
Member

PR 1/3 of the #1170 split.

# before
calculate_tiling_qc(sdata, "labels", distance_tol=1.0, min_area=30, max_contour_points=400)

# after
calculate_tiling_qc(sdata, "labels", tiling_qc_params={"distance_tol": 1.0, "min_area": 30, "max_contour_points": 400})
# or TilingQCParams(distance_tol=1.0, ...)

adata.uns["tiling_qc"] no longer carries flat distance_tol/min_area/max_contour_points; they live under ["tiling_qc_params"].

Also adds a private resolve_labels_array helper in experimental/utils/_labels.py (consumed here, reused by PR-B/PR-C). A strict variant of the existing get_element_data — fail-loud rather than fall-back, so downstream label pipelines can't silently pick the wrong scale.

PR-B will add stitch_tile_cuts + StitchParams and the QC-side warning hook. PR-C will add make_stitched_labels.

anon added 2 commits May 27, 2026 23:25
Collapse three tuning kwargs (distance_tol, min_area, max_contour_points)
on calculate_tiling_qc into a single tiling_qc_params bundle backed by a
TilingQCParams dataclass with validation and numpy-scalar coercion.
adata.uns["tiling_qc"] nests them under "tiling_qc_params" instead of
storing them flat.

Introduce experimental/utils/_labels.py with resolve_labels_array as a
private helper for the QC's multi-scale branch (and future labels
pipelines).

Breaking: drops the three flat kwargs and changes the uns schema; parent
feature scverse#1157 merged unreleased so no deprecation shim.
- Freeze TilingQCParams so post-construction mutation cannot bypass
  __post_init__ validation
- Return the _QC_DEFAULTS singleton from _resolve_qc_params(None) instead
  of allocating a fresh instance; hoist the field-name set to module
  scope
- Move the "scale ignored for single-scale" warning into
  resolve_labels_array so the QC caller no longer duplicates the
  isinstance(xr.DataTree) dispatch
- Note in the helper docstring why we have a strict variant alongside
  the existing get_element_data
- Use backtick-quoted parameter names in validation error messages to
  match calculate_tiling_qc's own error style
- Add a regression test locking the multi-scale-without-scale ValueError
@timtreis timtreis requested a review from selmanozleyen May 27, 2026 21:42
@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.15686% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.68%. Comparing base (154abbf) to head (fcb75c1).

Files with missing lines Patch % Lines
src/squidpy/experimental/utils/_labels.py 69.23% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1188      +/-   ##
==========================================
+ Coverage   74.52%   74.68%   +0.15%     
==========================================
  Files          51       52       +1     
  Lines        7581     7619      +38     
  Branches     1270     1277       +7     
==========================================
+ Hits         5650     5690      +40     
+ Misses       1422     1420       -2     
  Partials      509      509              
Files with missing lines Coverage Δ
src/squidpy/experimental/tl/_tiling_qc.py 70.03% <100.00%> (+5.14%) ⬆️
src/squidpy/experimental/utils/_labels.py 69.23% <69.23%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

timtreis and others added 5 commits May 27, 2026 18:09
Sphinx builds with -W (warnings as errors); the :meth: role tried to
resolve __post_init__ as a documented method and failed, breaking the
Read the Docs build. Use a plain literal instead -- renders identically,
no target to resolve.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread src/squidpy/experimental/tl/_tiling_qc.py

@selmanozleyen selmanozleyen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a nice refactor.

@timtreis timtreis merged commit b984ecd into scverse:main May 29, 2026
12 of 13 checks passed
@timtreis timtreis deleted the feature/tiling-qc-params branch May 29, 2026 13:20
@selmanozleyen selmanozleyen added the release-ignore Exclude from generated release notes label Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-ignore Exclude from generated release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants