From 812faa338235ce41df5c70eb282cf698a7b72a24 Mon Sep 17 00:00:00 2001 From: fishidaho Date: Wed, 2 Sep 2026 17:04:40 -0700 Subject: [PATCH 1/3] Make post-selection normalization explicit and reachable A normalized view's statistics are computed once, at construction, over the whole array it wrapped. `__getitem__` then applies those parent statistics to whatever sub-block is asked for -- while its docstring described the result as though the block were self-consistently normalized ("the transform/centering formula is then applied to that small block directly"). It isn't, and the gap is not small. On a mixed population -- two cell types differing in read depth and marker expression -- indexing the view for one cell type differs from normalizing those cells by **52% relative Frobenius norm**. Read as "the normalized data for these cells", that silently fits downstream analysis to different data than the caller believes. Rather than redefine what indexing returns, this keeps the two operations and names them: - `__getitem__` stays a window into this view's matrix -- exactly `toarray()[key]` without materializing the full matrix -- and now says so, including what it is *not*. - `select(rows, cols)` is new: it renormalizes the selected sub-array on its own terms, equivalent to `arr[sel].normalized()`, and returns a view so it still composes with `@`/`toarray()`. Recomputation was deliberately not made the default for indexing, because it is not uniformly more correct: `row_scale` is a per-cell total over the columns present, so recomputing after a *gene* selection re-derives read depth from just those genes, which is usually wrong. Making the caller ask for it keeps that choice explicit; `select`'s docstring spells the caveat out. Choosing per-selection semantics properly is v0.4's selection-algebra work. `select` also fills a real gap: selecting rows of a VCSCArray is a minor-axis selection, which drops out as scipy with no `.normalized()` at all, so the raw-array route isn't uniformly available. Tests pin both contracts against a dense reference, and assert the two disagree by >10% on a realistic selection -- so the distinction can't be quietly collapsed by a later change. Co-Authored-By: Claude Sonnet 5 --- src/vsparse/_norm_common.py | 72 +++++++++++++-- tests/test_norm_selection.py | 170 +++++++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 5 deletions(-) create mode 100644 tests/test_norm_selection.py diff --git a/src/vsparse/_norm_common.py b/src/vsparse/_norm_common.py index 842611e..29c1da0 100644 --- a/src/vsparse/_norm_common.py +++ b/src/vsparse/_norm_common.py @@ -9,6 +9,15 @@ whole point of a "view" here is to avoid paying for that until (and unless) the caller actually asks for it. +Every statistic below is computed once, at construction, over the whole +wrapped array -- so a view is tied to the population it was built from. +Indexing one (:meth:`~NormalizedViewBase.__getitem__`) is a window into +*that* matrix and keeps those statistics; normalizing a subset on its own +terms is :meth:`~NormalizedViewBase.select`, or equivalently selecting on +the raw array before calling ``normalized()``. The distinction is not +cosmetic: for a selection whose read depth or expression profile differs +from the parent, the two differ by tens of percent. + What's precomputed, once, at construction: - ``row_scale``: per-row (cell) total raw counts, scaled to a median of 1 -- @@ -52,6 +61,8 @@ import numba import numpy as np +from vsparse._indexutils import is_full_slice as _is_full_slice + __all__ = ["NormalizedViewBase"] @@ -179,6 +190,11 @@ class NormalizedViewBase: Subclasses fix ``_format`` (``"csc"``/``"csr"``) and supply ``__matmul__``/``__rmatmul__`` wired to :mod:`vsparse._vcs_matmul`. + + Statistics are computed at construction over the whole array passed in, + which fixes what this view means: the normalization of *that* + population. :meth:`__getitem__` windows into it; :meth:`select` + renormalizes a subset on its own terms. """ _format: str @@ -252,16 +268,62 @@ def toarray(self) -> np.ndarray: ) return out + # -- selection --------------------------------------------------------------- + + def select(self, rows: Any = slice(None), cols: Any = slice(None)) -> Any: + """A normalized view of the selected sub-array, with statistics recomputed for it. + + This is *not* what indexing the view does. ``view[rows]`` is a + window into this view's matrix and keeps this view's statistics + (see :meth:`__getitem__`); ``view.select(rows)`` throws those away + and normalizes the selected cells on their own terms, exactly as + ``arr[rows].normalized()`` would. For a selection whose read depth + or expression profile differs from the parent -- picking one cell + type out of a mixed population, say -- the two differ by tens of + percent, and it's ``select`` that matches normalizing the selected + cells directly. + + Note what recomputation means for a *column* selection: ``row_scale`` + is a per-cell total over whatever columns are present, so selecting + genes here re-derives read depth from just those genes. If that + isn't what you want (it usually isn't -- depth is normally measured + across all genes), select the genes first and normalize after, or + select only rows here. + + Returns a view, not a dense array, so it still composes with + ``@``/:meth:`toarray`. + """ + # One axis at a time, so two index arrays select a sub-block rather + # than being broadcast against each other pointwise the way a single + # ``arr[rows, cols]`` would. + sub: Any = self._arr + if not _is_full_slice(rows): + sub = sub[_prep_key(rows), :] + if not _is_full_slice(cols): + sub = sub[:, _prep_key(cols)] + if not isinstance(sub, type(self._arr)): + # Selecting the minor axis drops out of the raw array as scipy. + sub = type(self._arr).from_scipy(sub) + return type(self)(sub) + # -- on-the-fly elementwise access ------------------------------------------ def __getitem__(self, key: Any) -> np.ndarray: - """Compute just the requested sub-block, on the fly, from the raw data. + """A window into *this* view's matrix, computed on the fly from the raw data. Only the raw counts for the requested rows/columns are ever - decompressed (via the underlying array's own indexing, which stays - compact for a major-axis-only slice); the transform/centering - formula is then applied to that small block directly, using the - precomputed per-row/per-column statistics -- never the full matrix. + decompressed; the transform/centering formula is then applied to + that small block directly -- never the full matrix. + + The statistics it applies are this view's own, computed over the + **whole** wrapped array at construction. That makes this exactly + ``toarray()[key]``, without materializing the full matrix -- and it + makes it emphatically *not* the same as normalizing the selected + sub-matrix, which would derive read depth, per-gene scale and + centering from the selection alone. On a selection that differs + from the parent population the two disagree by tens of percent. + Use :meth:`select` (or select on the raw array and normalize after) + when you want post-selection statistics. """ if isinstance(key, tuple): if len(key) != 2: diff --git a/tests/test_norm_selection.py b/tests/test_norm_selection.py new file mode 100644 index 0000000..3684428 --- /dev/null +++ b/tests/test_norm_selection.py @@ -0,0 +1,170 @@ +"""Windowing a normalized view vs. renormalizing a selection -- and why they differ. + +A normalized view's statistics are fixed at construction, over the whole +array it wrapped. Indexing it is a window into *that* matrix; ``select`` +renormalizes the chosen sub-array on its own terms. Getting those two +confused silently fits downstream analysis to the wrong data, so both +contracts are pinned here, along with the size of the gap between them. +""" + +from __future__ import annotations + +import numpy as np +import pytest +import scipy.sparse as sp + +from vsparse import VCSCArray, VCSCArrayNormalized, VCSRArray, VCSRArrayNormalized + + +@pytest.fixture(params=[VCSCArray, VCSRArray]) +def vcls(request): + return request.param + + +def _scipy_for(vcls, dense): + return sp.csc_array(dense) if vcls is VCSCArray else sp.csr_array(dense) + + +def _norm_cls(vcls): + return VCSCArrayNormalized if vcls is VCSCArray else VCSRArrayNormalized + + +def _reference(dense: np.ndarray) -> np.ndarray: + """Read-depth normalize, log-transform, and mean-center a dense matrix directly.""" + row_totals = dense.sum(axis=1) + row_scale = row_totals / np.median(row_totals) + row_scale[row_scale == 0.0] = 1.0 + scaled = dense / row_scale[:, None] + gene_scale = scaled.sum(axis=0) + with np.errstate(divide="ignore", invalid="ignore"): + normalized = np.where(gene_scale > 0, scaled / gene_scale[None, :], 0.0) + transformed = np.log10(1.0 + 1000.0 * normalized) + return transformed - transformed.mean(axis=0, keepdims=True) + + +def _mixed_population(seed: int = 0) -> tuple[np.ndarray, np.ndarray]: + """Two cell types: different sequencing depth, different marker genes. + + This is the shape of data the distinction actually matters for -- a + selection whose depth and expression profile both differ from the + population the view was built over. + """ + rng = np.random.default_rng(seed) + n_cells, n_genes = 240, 80 + dense = np.empty((n_cells, n_genes)) + dense[: n_cells // 2] = rng.poisson(0.6, size=(n_cells // 2, n_genes)) + dense[n_cells // 2 :] = rng.poisson(4.0, size=(n_cells // 2, n_genes)) + dense[n_cells // 2 :, :16] *= 6 # markers expressed only in the second type + + mask = np.zeros(n_cells, dtype=bool) + mask[n_cells // 2 :] = True + return dense, mask + + +def _relative_frobenius(got: np.ndarray, want: np.ndarray) -> float: + return float(np.linalg.norm(got - want) / np.linalg.norm(want)) + + +# -- select(): statistics recomputed for the selection ----------------------- + + +def test_select_matches_normalizing_the_selection_directly(vcls): + """The correctness claim: select() == normalizing those cells on their own.""" + dense, mask = _mixed_population() + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + + got = nv.select(mask).toarray() + want = _reference(dense[mask]) + + assert _relative_frobenius(got, want) < 1e-12 + np.testing.assert_allclose(got, want, atol=1e-10) + + +def test_select_equals_selecting_on_the_raw_array_first(vcls): + """select() is exactly the arr[sel].normalized() route, reachable from the view. + + Note the raw route isn't uniformly available: selecting rows of a + VCSCArray is a *minor*-axis selection, which drops out as scipy and has + no ``.normalized()`` at all. ``select`` works either way. + """ + dense, mask = _mixed_population() + arr = vcls.from_scipy(_scipy_for(vcls, dense)) + + raw_sub = arr[mask, :] + if not isinstance(raw_sub, vcls): + raw_sub = vcls.from_scipy(raw_sub) + + np.testing.assert_allclose( + arr.normalized().select(mask).toarray(), + raw_sub.normalized().toarray(), + atol=1e-12, + ) + + +def test_select_returns_a_view_that_still_composes(vcls): + """Not a dense block: it keeps working with @ and toarray().""" + dense, mask = _mixed_population() + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + + sub = nv.select(mask) + assert isinstance(sub, _norm_cls(vcls)) + assert sub.shape == (int(mask.sum()), dense.shape[1]) + + rng = np.random.default_rng(3) + B = rng.normal(size=(dense.shape[1], 4)) + np.testing.assert_allclose(sub @ B, _reference(dense[mask]) @ B, atol=1e-8) + + +def test_select_columns_and_both_axes(vcls): + dense, _ = _mixed_population() + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + cols = np.arange(0, dense.shape[1], 3) + + np.testing.assert_allclose( + nv.select(cols=cols).toarray(), _reference(dense[:, cols]), atol=1e-10 + ) + + rows = np.arange(0, dense.shape[0], 5) + np.testing.assert_allclose( + nv.select(rows, cols).toarray(), _reference(dense[np.ix_(rows, cols)]), atol=1e-10 + ) + + +def test_select_everything_is_the_whole_view(vcls, dense): + if dense.sum() == 0: + pytest.skip("all-zero matrix: median row total is 0") + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + np.testing.assert_allclose(nv.select().toarray(), nv.toarray(), atol=1e-12) + + +# -- __getitem__: a window that keeps the parent's statistics ---------------- + + +def test_getitem_is_a_window_into_the_parent_matrix(vcls): + """Indexing == toarray()[key], without materializing the full matrix.""" + dense, mask = _mixed_population() + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + + full = nv.toarray() + np.testing.assert_allclose(nv[mask, :], full[mask, :], atol=1e-10) + np.testing.assert_allclose(nv[0:4, 0:4], full[0:4, 0:4], atol=1e-10) + + +def test_getitem_and_select_disagree_substantially_on_a_real_selection(vcls): + """The hazard, made explicit and measurable. + + Reading ``view[cell_type]`` as "the normalized data for these cells" is + wrong by tens of percent -- large enough that any factorization fit to + it is fitting different data than the caller thinks. This test exists so + that the difference can't be quietly collapsed by a future change: if + these two ever agree, one of the two contracts has been broken. + """ + dense, mask = _mixed_population() + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + + want = _reference(dense[mask]) + windowed = np.asarray(nv[mask, :]) + renormalized = nv.select(mask).toarray() + + assert _relative_frobenius(renormalized, want) < 1e-12 + assert _relative_frobenius(windowed, want) > 0.1 From 99af78af2c4eab556bd40138cebff116a5dfaf1a Mon Sep 17 00:00:00 2001 From: fishidaho Date: Thu, 3 Sep 2026 11:16:25 -0700 Subject: [PATCH 2/3] Simplify select() onto #23's native two-axis indexing #23 changed what a raw array's __getitem__ returns underneath this branch, in two ways that matter here: - A minor-axis selection now stays VCS-native instead of falling out as a scipy array. `select`'s re-wrap is therefore no longer the normal path, just a defensive one -- reworded to match the same defensive re-wrap in `_anndata_class._subset_2d`, and the test comment claiming a VCSCArray row selection "has no .normalized() at all" is simply no longer true. - `arr[rows, cols]` with two index arrays now composes a major- and a minor-axis selection, which is the sub-block semantics `select` wants. It previously fell through to scipy, which broadcasts two index arrays pointwise, and that's why this was written one axis at a time. That workaround is now redundant, so it collapses to a single index operation. Adds a test pinning the outer-vs-pointwise distinction directly, using row and column selections of different lengths -- a pointwise broadcast would fail outright rather than return something subtly wrong, which is what makes this worth keeping a test on. Co-Authored-By: Claude Sonnet 5 --- src/vsparse/_norm_common.py | 15 ++++----------- tests/test_norm_selection.py | 29 ++++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/vsparse/_norm_common.py b/src/vsparse/_norm_common.py index 29c1da0..f21a2c2 100644 --- a/src/vsparse/_norm_common.py +++ b/src/vsparse/_norm_common.py @@ -61,8 +61,6 @@ import numba import numpy as np -from vsparse._indexutils import is_full_slice as _is_full_slice - __all__ = ["NormalizedViewBase"] @@ -293,16 +291,11 @@ def select(self, rows: Any = slice(None), cols: Any = slice(None)) -> Any: Returns a view, not a dense array, so it still composes with ``@``/:meth:`toarray`. """ - # One axis at a time, so two index arrays select a sub-block rather - # than being broadcast against each other pointwise the way a single - # ``arr[rows, cols]`` would. - sub: Any = self._arr - if not _is_full_slice(rows): - sub = sub[_prep_key(rows), :] - if not _is_full_slice(cols): - sub = sub[:, _prep_key(cols)] + sub: Any = self._arr[_prep_key(rows), _prep_key(cols)] if not isinstance(sub, type(self._arr)): - # Selecting the minor axis drops out of the raw array as scipy. + # Defensive, as in vsparse._anndata_class._subset_2d: the raw + # array's __getitem__ only leaves the VCS types when both axes + # collapse to a scalar, which _prep_key rules out above. sub = type(self._arr).from_scipy(sub) return type(self)(sub) diff --git a/tests/test_norm_selection.py b/tests/test_norm_selection.py index 3684428..424f2ee 100644 --- a/tests/test_norm_selection.py +++ b/tests/test_norm_selection.py @@ -83,16 +83,15 @@ def test_select_matches_normalizing_the_selection_directly(vcls): def test_select_equals_selecting_on_the_raw_array_first(vcls): """select() is exactly the arr[sel].normalized() route, reachable from the view. - Note the raw route isn't uniformly available: selecting rows of a - VCSCArray is a *minor*-axis selection, which drops out as scipy and has - no ``.normalized()`` at all. ``select`` works either way. + Since #23 the raw route stays VCS-native on both axes, so this is now a + direct comparison; ``select`` remains the discoverable spelling, and the + one that doesn't require reaching for the view's private ``_arr``. """ dense, mask = _mixed_population() arr = vcls.from_scipy(_scipy_for(vcls, dense)) raw_sub = arr[mask, :] - if not isinstance(raw_sub, vcls): - raw_sub = vcls.from_scipy(raw_sub) + assert isinstance(raw_sub, vcls) np.testing.assert_allclose( arr.normalized().select(mask).toarray(), @@ -168,3 +167,23 @@ def test_getitem_and_select_disagree_substantially_on_a_real_selection(vcls): assert _relative_frobenius(renormalized, want) < 1e-12 assert _relative_frobenius(windowed, want) > 0.1 + + +def test_select_survives_native_two_axis_indexing(vcls): + """#23 made both-axes selection VCS-native; select() must still get outer semantics. + + A single ``arr[rows, cols]`` used to fall through to scipy, which + broadcasts two index arrays against each other pointwise. It now + composes a major- and a minor-axis selection instead, which is the + sub-block ``select`` needs -- worth pinning, since the difference is + silent and only shows up when both axes are arrays. + """ + dense, _ = _mixed_population() + nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() + rows = np.arange(0, dense.shape[0], 7) + cols = np.arange(0, dense.shape[1], 5) + assert rows.shape != cols.shape # a pointwise broadcast would fail outright + + np.testing.assert_allclose( + nv.select(rows, cols).toarray(), _reference(dense[np.ix_(rows, cols)]), atol=1e-10 + ) From 72519f6c920b5029b64a44df3914fe0445a7e4c4 Mon Sep 17 00:00:00 2001 From: fishidaho Date: Thu, 3 Sep 2026 17:07:43 -0700 Subject: [PATCH 3/3] Trim comments and tests Docstrings state what the code does. The two both-axes selection tests fold into one that uses different-length keys, which is what actually pins outer semantics. --- src/vsparse/_norm_common.py | 58 ++++++-------------------------- tests/test_norm_selection.py | 65 ++++++------------------------------ 2 files changed, 20 insertions(+), 103 deletions(-) diff --git a/src/vsparse/_norm_common.py b/src/vsparse/_norm_common.py index f21a2c2..e02eac3 100644 --- a/src/vsparse/_norm_common.py +++ b/src/vsparse/_norm_common.py @@ -9,14 +9,9 @@ whole point of a "view" here is to avoid paying for that until (and unless) the caller actually asks for it. -Every statistic below is computed once, at construction, over the whole -wrapped array -- so a view is tied to the population it was built from. -Indexing one (:meth:`~NormalizedViewBase.__getitem__`) is a window into -*that* matrix and keeps those statistics; normalizing a subset on its own -terms is :meth:`~NormalizedViewBase.select`, or equivalently selecting on -the raw array before calling ``normalized()``. The distinction is not -cosmetic: for a selection whose read depth or expression profile differs -from the parent, the two differ by tens of percent. +Statistics are computed once, at construction, over the whole wrapped +array. Indexing a view windows that matrix and keeps those statistics; +:meth:`~NormalizedViewBase.select` renormalizes a subset on its own terms. What's precomputed, once, at construction: @@ -188,11 +183,6 @@ class NormalizedViewBase: Subclasses fix ``_format`` (``"csc"``/``"csr"``) and supply ``__matmul__``/``__rmatmul__`` wired to :mod:`vsparse._vcs_matmul`. - - Statistics are computed at construction over the whole array passed in, - which fixes what this view means: the normalization of *that* - population. :meth:`__getitem__` windows into it; :meth:`select` - renormalizes a subset on its own terms. """ _format: str @@ -271,52 +261,24 @@ def toarray(self) -> np.ndarray: def select(self, rows: Any = slice(None), cols: Any = slice(None)) -> Any: """A normalized view of the selected sub-array, with statistics recomputed for it. - This is *not* what indexing the view does. ``view[rows]`` is a - window into this view's matrix and keeps this view's statistics - (see :meth:`__getitem__`); ``view.select(rows)`` throws those away - and normalizes the selected cells on their own terms, exactly as - ``arr[rows].normalized()`` would. For a selection whose read depth - or expression profile differs from the parent -- picking one cell - type out of a mixed population, say -- the two differ by tens of - percent, and it's ``select`` that matches normalizing the selected - cells directly. - - Note what recomputation means for a *column* selection: ``row_scale`` - is a per-cell total over whatever columns are present, so selecting - genes here re-derives read depth from just those genes. If that - isn't what you want (it usually isn't -- depth is normally measured - across all genes), select the genes first and normalize after, or - select only rows here. - Returns a view, not a dense array, so it still composes with ``@``/:meth:`toarray`. """ + # A column selection re-derives read depth from only the selected + # columns, which is rarely what a caller wants; select genes first + # and normalize after if it matters. sub: Any = self._arr[_prep_key(rows), _prep_key(cols)] if not isinstance(sub, type(self._arr)): - # Defensive, as in vsparse._anndata_class._subset_2d: the raw - # array's __getitem__ only leaves the VCS types when both axes - # collapse to a scalar, which _prep_key rules out above. sub = type(self._arr).from_scipy(sub) return type(self)(sub) # -- on-the-fly elementwise access ------------------------------------------ def __getitem__(self, key: Any) -> np.ndarray: - """A window into *this* view's matrix, computed on the fly from the raw data. - - Only the raw counts for the requested rows/columns are ever - decompressed; the transform/centering formula is then applied to - that small block directly -- never the full matrix. - - The statistics it applies are this view's own, computed over the - **whole** wrapped array at construction. That makes this exactly - ``toarray()[key]``, without materializing the full matrix -- and it - makes it emphatically *not* the same as normalizing the selected - sub-matrix, which would derive read depth, per-gene scale and - centering from the selection alone. On a selection that differs - from the parent population the two disagree by tens of percent. - Use :meth:`select` (or select on the raw array and normalize after) - when you want post-selection statistics. + """``toarray()[key]``, computed on the fly without materializing the full matrix. + + Applies this view's statistics, so it is not the same as normalizing + the selected sub-matrix; use :meth:`select` for that. """ if isinstance(key, tuple): if len(key) != 2: diff --git a/tests/test_norm_selection.py b/tests/test_norm_selection.py index 424f2ee..bd3dbb3 100644 --- a/tests/test_norm_selection.py +++ b/tests/test_norm_selection.py @@ -1,12 +1,3 @@ -"""Windowing a normalized view vs. renormalizing a selection -- and why they differ. - -A normalized view's statistics are fixed at construction, over the whole -array it wrapped. Indexing it is a window into *that* matrix; ``select`` -renormalizes the chosen sub-array on its own terms. Getting those two -confused silently fits downstream analysis to the wrong data, so both -contracts are pinned here, along with the size of the gap between them. -""" - from __future__ import annotations import numpy as np @@ -43,12 +34,7 @@ def _reference(dense: np.ndarray) -> np.ndarray: def _mixed_population(seed: int = 0) -> tuple[np.ndarray, np.ndarray]: - """Two cell types: different sequencing depth, different marker genes. - - This is the shape of data the distinction actually matters for -- a - selection whose depth and expression profile both differ from the - population the view was built over. - """ + """Two cell types differing in both sequencing depth and marker genes.""" rng = np.random.default_rng(seed) n_cells, n_genes = 240, 80 dense = np.empty((n_cells, n_genes)) @@ -69,7 +55,7 @@ def _relative_frobenius(got: np.ndarray, want: np.ndarray) -> float: def test_select_matches_normalizing_the_selection_directly(vcls): - """The correctness claim: select() == normalizing those cells on their own.""" + """select() gives the same answer as normalizing those cells on their own.""" dense, mask = _mixed_population() nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() @@ -81,12 +67,7 @@ def test_select_matches_normalizing_the_selection_directly(vcls): def test_select_equals_selecting_on_the_raw_array_first(vcls): - """select() is exactly the arr[sel].normalized() route, reachable from the view. - - Since #23 the raw route stays VCS-native on both axes, so this is now a - direct comparison; ``select`` remains the discoverable spelling, and the - one that doesn't require reaching for the view's private ``_arr``. - """ + """select() matches selecting on the raw array and normalizing after.""" dense, mask = _mixed_population() arr = vcls.from_scipy(_scipy_for(vcls, dense)) @@ -101,7 +82,7 @@ def test_select_equals_selecting_on_the_raw_array_first(vcls): def test_select_returns_a_view_that_still_composes(vcls): - """Not a dense block: it keeps working with @ and toarray().""" + """The result is a view, so it still composes with @ and toarray().""" dense, mask = _mixed_population() nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() @@ -115,15 +96,16 @@ def test_select_returns_a_view_that_still_composes(vcls): def test_select_columns_and_both_axes(vcls): + """Two index arrays select a sub-block, not a pointwise diagonal.""" dense, _ = _mixed_population() nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() - cols = np.arange(0, dense.shape[1], 3) + rows = np.arange(0, dense.shape[0], 7) + cols = np.arange(0, dense.shape[1], 5) + assert rows.shape != cols.shape np.testing.assert_allclose( nv.select(cols=cols).toarray(), _reference(dense[:, cols]), atol=1e-10 ) - - rows = np.arange(0, dense.shape[0], 5) np.testing.assert_allclose( nv.select(rows, cols).toarray(), _reference(dense[np.ix_(rows, cols)]), atol=1e-10 ) @@ -140,7 +122,7 @@ def test_select_everything_is_the_whole_view(vcls, dense): def test_getitem_is_a_window_into_the_parent_matrix(vcls): - """Indexing == toarray()[key], without materializing the full matrix.""" + """Indexing gives the same values as the fully materialized view.""" dense, mask = _mixed_population() nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() @@ -150,14 +132,7 @@ def test_getitem_is_a_window_into_the_parent_matrix(vcls): def test_getitem_and_select_disagree_substantially_on_a_real_selection(vcls): - """The hazard, made explicit and measurable. - - Reading ``view[cell_type]`` as "the normalized data for these cells" is - wrong by tens of percent -- large enough that any factorization fit to - it is fitting different data than the caller thinks. This test exists so - that the difference can't be quietly collapsed by a future change: if - these two ever agree, one of the two contracts has been broken. - """ + """Windowing and renormalizing diverge by tens of percent on a real selection.""" dense, mask = _mixed_population() nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() @@ -167,23 +142,3 @@ def test_getitem_and_select_disagree_substantially_on_a_real_selection(vcls): assert _relative_frobenius(renormalized, want) < 1e-12 assert _relative_frobenius(windowed, want) > 0.1 - - -def test_select_survives_native_two_axis_indexing(vcls): - """#23 made both-axes selection VCS-native; select() must still get outer semantics. - - A single ``arr[rows, cols]`` used to fall through to scipy, which - broadcasts two index arrays against each other pointwise. It now - composes a major- and a minor-axis selection instead, which is the - sub-block ``select`` needs -- worth pinning, since the difference is - silent and only shows up when both axes are arrays. - """ - dense, _ = _mixed_population() - nv = vcls.from_scipy(_scipy_for(vcls, dense)).normalized() - rows = np.arange(0, dense.shape[0], 7) - cols = np.arange(0, dense.shape[1], 5) - assert rows.shape != cols.shape # a pointwise broadcast would fail outright - - np.testing.assert_allclose( - nv.select(rows, cols).toarray(), _reference(dense[np.ix_(rows, cols)]), atol=1e-10 - )