feat: stalign api and landmark alignment - #1195
Open
selmanozleyen wants to merge 267 commits into
Open
Conversation
for more information, see https://pre-commit.ci
…leyen/squidpy into feat/spatial_neighbours
for more information, see https://pre-commit.ci
…leyen/squidpy into feat/spatial_neighbours
for more information, see https://pre-commit.ci
…leyen/squidpy into feat/spatial_neighbours
Both were closed vocabularies of strings that callers write as strings, and neither earned the class it was declared as. `DetectTissueMethod` used `enum.auto()`, so its members carried no value anyone read; every use was an equality test, and a string argument was coerced through `DetectTissueMethod[method.upper()]` before any of them ran. It was never exported either, while its own docstring told callers to pass `DetectTissueMethod.OTSU` -- advice they could not follow. The `.upper()` in that coercion is why `method` is lowered before the new membership check: `"OTSU"` was accepted before and still is. `QCMetric` was a `StrEnum`, so its members already were their own strings and worked as registry keys unchanged. Its validation, though, was `isinstance(m, QCMetric)`, which is False for the plain string the member compares equal to: `qc_image(metrics="tenengrad")` raised while the docs advertised a `StrEnum`. Both now validate against `get_args`, which accepts what callers actually write. Two behaviour changes worth naming. An unknown metric now raises `ValueError` rather than `TypeError` -- it is a bad value, not a bad type, and the old class was chosen to complain about enum membership. And `QCMetric` leaves the module namespace entirely, not just `__all__`: a page rendering as nothing but `alias of Literal[...]` documents an argument rather than a type callers hold, and the names it listed now sit on the parameter that takes them, in both `im.qc_image` and its `pl` counterpart. `InputKind` stays an enum. It is internal and never crosses the public boundary.
The API page was one flat list per area, every line repeating the module it belonged to, and `experimental` was a single block interleaving `im`, `tl` and `pl`. Group it: every section names its module, `experimental` splits by submodule and then by what the entries are for, and `neighbors` moves under Graph so `GraphMatrixT` is documented once rather than beside the `gr` functions, where a bare type variable read as public API. `squidpy.types` gains the two result tuples alongside the parameter bags, and the params leave `im`/`tl`'s `__all__` so it is the single public route to them. Nine names were public but absent from the page, among them `detect_tissue`, `make_tiles` and `qc_image`. Docs machinery, so the above renders: attributes inline with their types rather than an untyped summary table, `navigation_depth` at 5 so a section unfolds to its pages instead of stopping at the sub-section, and page titles as the bare name rather than the dotted path repeated in every nav entry. `typeddict.rst` goes: it was byte-identical to the built-in `base.rst` it shadowed, so it rendered nothing the default did not.
# Conflicts: # docs/api.md # src/squidpy/experimental/im/__init__.py # src/squidpy/experimental/tl/__init__.py # src/squidpy/types.py
Public classes in `experimental` declare their kind in the suffix: `*Fit` is an output carrying the operations that apply it, `*Params` is inert input the caller fills in. With `transform` and `decompose` on it, this is a fit by that definition -- and it was the only output class left without the marker, so the index read as if it were a third kind of thing. Deliberately not renamed before this commit: without behaviour it was arguably just a parameter bundle, and the methods are what make `*Fit` honest. `fit_stain_reference() -> StainFit` leaves the verb naming the operation and the noun naming what comes back. The function keeps its name: it fits a stain *reference*, which is what the object is; the suffix says what kind of thing it is, not what it models. No deprecation alias: the module is experimental and says so.
`eq=False` and the explicit `__eq__`/`__hash__` went out with the commit that added the
methods, which left `==` and `hash()` raising on a class that shipped in v1.8.3:
a == b ValueError: truth value of an array ... is ambiguous
hash(a) TypeError: unhashable type: 'numpy.ndarray'
a in [b] ValueError
The dataclass-generated `__eq__` compares field tuples, and comparing array fields is what
raises. `a == a` still answers True by identity short-circuit, so a smoke test passes while
`fit in cohort_fits` blows up on the first non-identical element.
The test that replaced the old one asserted the raising as intended, on the grounds that the
deleted code "silently answered by identity". It did not: it compared `method` plus
element-wise arrays, and the test it replaced asserted exactly that two distinct fits with
equal arrays compare equal. Identity was only ever the *hash*, deliberately, because array
fields cannot produce a value-based one.
Both were closed vocabularies of strings that callers write as strings, and neither earned the class it was declared as. `DetectTissueMethod` used `enum.auto()`, so its members carried no value anyone read; every use was an equality test, and a string argument was coerced through `DetectTissueMethod[method.upper()]` before any of them ran. It was never exported either, while its own docstring told callers to pass `DetectTissueMethod.OTSU` -- advice they could not follow. The `.upper()` in that coercion is why `method` is lowered before the new membership check: `"OTSU"` was accepted before and still is. `QCMetric` was a `StrEnum`, so its members already were their own strings and worked as registry keys unchanged. Its validation, though, was `isinstance(m, QCMetric)`, which is False for the plain string the member compares equal to: `qc_image(metrics="tenengrad")` raised while the docs advertised a `StrEnum`. Both now validate against `get_args`, which accepts what callers actually write. Two behaviour changes worth naming. An unknown metric now raises `ValueError` rather than `TypeError` -- it is a bad value, not a bad type, and the old class was chosen to complain about enum membership. And `QCMetric` leaves the module namespace entirely, not just `__all__`: a page rendering as nothing but `alias of Literal[...]` documents an argument rather than a type callers hold, and the names it listed now sit on the parameter that takes them, in both `im.qc_image` and its `pl` counterpart. `InputKind` stays an enum. It is internal and never crosses the public boundary.
The API page was one flat list per area, every line repeating the module it belonged to, and `experimental` was a single block interleaving `im`, `tl` and `pl`. Group it: every section names its module, `experimental` splits by submodule and then by what the entries are for, and `neighbors` moves under Graph so `GraphMatrixT` is documented once rather than beside the `gr` functions, where a bare type variable read as public API. `squidpy.types` gains the two result tuples alongside the parameter bags, and the params leave `im`/`tl`'s `__all__` so it is the single public route to them. Nine names were public but absent from the page, among them `detect_tissue`, `make_tiles` and `qc_image`. Docs machinery, so the above renders: attributes inline with their types rather than an untyped summary table, `navigation_depth` at 5 so a section unfolds to its pages instead of stopping at the sub-section, and page titles as the bare name rather than the dotted path repeated in every nav entry. `typeddict.rst` goes: it was byte-identical to the built-in `base.rst` it shadowed, so it rendered nothing the default did not.
# Conflicts: # docs/api.md # src/squidpy/experimental/im/__init__.py # src/squidpy/experimental/tl/__init__.py # src/squidpy/types.py
Public classes in `experimental` declare their kind in the suffix: `*Fit` is an output carrying the operations that apply it, `*Params` is inert input the caller fills in. With `transform` and `decompose` on it, this is a fit by that definition -- and it was the only output class left without the marker, so the index read as if it were a third kind of thing. Deliberately not renamed before this commit: without behaviour it was arguably just a parameter bundle, and the methods are what make `*Fit` honest. `fit_stain_reference() -> StainFit` leaves the verb naming the operation and the noun naming what comes back. The function keeps its name: it fits a stain *reference*, which is what the object is; the suffix says what kind of thing it is, not what it models. No deprecation alias: the module is experimental and says so.
`eq=False` and the explicit `__eq__`/`__hash__` went out with the commit that added the
methods, which left `==` and `hash()` raising on a class that shipped in v1.8.3:
a == b ValueError: truth value of an array ... is ambiguous
hash(a) TypeError: unhashable type: 'numpy.ndarray'
a in [b] ValueError
The dataclass-generated `__eq__` compares field tuples, and comparing array fields is what
raises. `a == a` still answers True by identity short-circuit, so a smoke test passes while
`fit in cohort_fits` blows up on the first non-identical element.
The test that replaced the old one asserted the raising as intended, on the grounds that the
deleted code "silently answered by identity". It did not: it compared `method` plus
element-wise arrays, and the test it replaced asserted exactly that two distinct fits with
equal arrays compare equal. Identity was only ever the *hash*, deliberately, because array
fields cannot produce a value-based one.
Both were closed vocabularies of strings that callers write as strings, and neither earned the class it was declared as. `DetectTissueMethod` used `enum.auto()`, so its members carried no value anyone read; every use was an equality test, and a string argument was coerced through `DetectTissueMethod[method.upper()]` before any of them ran. It was never exported either, while its own docstring told callers to pass `DetectTissueMethod.OTSU` -- advice they could not follow. The `.upper()` in that coercion is why `method` is lowered before the new membership check: `"OTSU"` was accepted before and still is. `QCMetric` was a `StrEnum`, so its members already were their own strings and worked as registry keys unchanged. Its validation, though, was `isinstance(m, QCMetric)`, which is False for the plain string the member compares equal to: `qc_image(metrics="tenengrad")` raised while the docs advertised a `StrEnum`. Both now validate against `get_args`, which accepts what callers actually write. Two behaviour changes worth naming. An unknown metric now raises `ValueError` rather than `TypeError` -- it is a bad value, not a bad type, and the old class was chosen to complain about enum membership. And `QCMetric` leaves the module namespace entirely, not just `__all__`: a page rendering as nothing but `alias of Literal[...]` documents an argument rather than a type callers hold, and the names it listed now sit on the parameter that takes them, in both `im.qc_image` and its `pl` counterpart. `InputKind` stays an enum. It is internal and never crosses the public boundary.
The API page was one flat list per area, every line repeating the module it belonged to, and `experimental` was a single block interleaving `im`, `tl` and `pl`. Group it: every section names its module, `experimental` splits by submodule and then by what the entries are for, and `neighbors` moves under Graph so `GraphMatrixT` is documented once rather than beside the `gr` functions, where a bare type variable read as public API. `squidpy.types` gains the two result tuples alongside the parameter bags, and the params leave `im`/`tl`'s `__all__` so it is the single public route to them. Nine names were public but absent from the page, among them `detect_tissue`, `make_tiles` and `qc_image`. Docs machinery, so the above renders: attributes inline with their types rather than an untyped summary table, `navigation_depth` at 5 so a section unfolds to its pages instead of stopping at the sub-section, and page titles as the bare name rather than the dotted path repeated in every nav entry. `typeddict.rst` goes: it was byte-identical to the built-in `base.rst` it shadowed, so it rendered nothing the default did not.
The Python domain renders a typed field inline as ``name (type) - description`` inside a two-column grid, so the three things a reader scans for share one run-on line indented behind the "Parameters:" label. A doctree transform splits each entry into ``name : type`` and its prose, and the field list is laid out as blocks rather than a grid. ``typehints_defaults`` puts each default next to its type. The signature line gets the name at a size worth landing on, with the module path receding behind it.
selmanozleyen
force-pushed
the
feat/api-docs-restructure
branch
from
August 30, 2026 22:09
f2ad7e1 to
8979a1b
Compare
# Conflicts: # docs/api.md # docs/conf.py # src/squidpy/experimental/im/__init__.py # src/squidpy/experimental/tl/__init__.py
selmanozleyen
force-pushed
the
feat/api-docs-restructure
branch
from
August 30, 2026 22:27
8979a1b to
c758c20
Compare
# Conflicts: # docs/api.md # docs/conf.py # src/squidpy/experimental/im/__init__.py # src/squidpy/experimental/tl/__init__.py
``apply_affine`` carried no ``Parameters`` section at all, so neither ``matrix`` nor ``points`` was documented and the page showed a bare return type. Drop the narrative in both it and the landmark arguments, and say what the arguments are.
for more information, see https://pre-commit.ci
``pl.qc_image`` respelled every type the annotation already gives and named its return twice; ``tl.make_stitched_labels`` and ``pl.tiling_qc`` documented no return at all. Each parameter now renders its own ``(default: x)``, so the inline ``(default)`` markers duplicate it -- the computed ones, which no signature can show, stay. ``QCMetric`` is a fifteen-value alias that ``qc_image`` spelled out twice; it renders by name.
… feat/experimental-fit-core
``pl.qc_image`` respelled every type the annotation already gives and named its return twice; ``tl.make_stitched_labels`` and ``pl.tiling_qc`` documented no return at all. Each parameter now renders its own ``(default: x)``, so the inline ``(default)`` markers duplicate it -- the computed ones, which no signature can show, stay. ``QCMetric`` is a fifteen-value alias that ``qc_image`` spelled out twice; it renders by name.
selmanozleyen
force-pushed
the
feat/api-docs-restructure
branch
from
August 30, 2026 23:20
3c54e85 to
7b33737
Compare
The classes became ``*Params`` but the keyword they arrive through stayed ``solver_kwargs``, the only ``Unpack`` site in the package not named for what it carries -- ``**tiling_qc_params``, ``**stitch_params`` and ``**background_detection_params`` all are.
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
3 participants
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.
Continuation of #1162