Skip to content

Sparse-array type check in fit()/center_data() misses valid subtypes (crash or silent wrong branch) #3

Description

@mkb-phoenix

Where: cellpie_main.py, intNMF.fit(), line 109; and cp_utils.py, center_data(), line 36 (identical pattern in both places).

if type(adata.X).__name__ in ['csr_matrix', 'SparseCSRView']:
    self.gene_expr = pd.DataFrame(data = adata.X.toarray(), index = adata.obs.index, columns=adata.var_names)
else:
    self.gene_expr = pd.DataFrame(data = adata.X, index = adata.obs.index, columns=adata.var_names)

Symptom: when adata.X is a sparse array whose exact class name isn't one of the two hardcoded strings, execution silently falls into the else branch and hands pd.DataFrame a sparse-matrix object where it expects a dense array. Depending on the installed anndata/scipy versions, this raises something like ValueError: Shape of passed values is (N, 1), indices imply (N, M) — a confusing error far from the actual cause.

Root cause: the check is a hardcoded list of two exact class-name strings, not a structural ("is this sparse?") test. It happens to work for a plain scipy.sparse.csr_matrix or anndata's own SparseCSRView, but a double-indexed view — adata[rows, :][:, cols], exactly what a bi-cross-validation block split produces — comes back under a different class name depending on the anndata/scipy versions installed. The class-name string isn't part of any public API contract, so this check is inherently version-fragile.

Minimal repro condition: take any AnnData object with a sparse .X, slice it on both axes (adata[np.array(row_idx), :][:, np.array(col_idx)]), and pass the result into intNMF.fit() or cp_utils.center_data() on an anndata/scipy combination newer than whatever was last validated against.

Suggested fix (duck-typing, version-independent):

if hasattr(adata.X, "toarray"):
    self.gene_expr = pd.DataFrame(data = adata.X.toarray(), index = adata.obs.index, columns=adata.var_names)
else:
    self.gene_expr = pd.DataFrame(data = adata.X, index = adata.obs.index, columns=adata.var_names)

Found while running CellPie's own cp_utils.model_selection() bi-cross-validation at production scale (700K–1.5M spatial units per section, ~2 orders of magnitude above the example notebooks' spot counts) — the block-splitting BCV does exactly this double-indexed-slice operation. Happy to share our patched fork if useful (commit pinned at 8880d673c, main branch).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions