Skip to content

feat: switch to use and out - #4359

Draft
flying-sheep wants to merge 6 commits into
mainfrom
fields-acc
Draft

flying-sheep wants to merge 6 commits into
mainfrom
fields-acc

Conversation

@flying-sheep

@flying-sheep flying-sheep commented Sep 11, 2026

Copy link
Copy Markdown
Member

Current design:

  • deprecate copy everywhere
  • functions operating on a main matrix get use and out – default for out is to operate in place (out=False), while out=True means returning the matrix instead of writing it into AnnData.
  • multi-in/out functions use key_added/inplace for the same functionality, like Neighbors(dists=A.obsp["foo"], conns=A.obsp["bar"], meta=A.uns["foo"]) (where A.uns doesn’t even exist yet)
    • Variant: create NamedTuples or so for the spots used. Probably what @ilan-gold would prefer, but we should meet to spec this out.

Examples for use/out (key_added is unchanged):

# in-place on X
sc.pp.log1p(adata)
# read counts, write X
sc.pp.log1p(adata, use=A.layers["counts"], out=A.X)
# return numpy array, equivalent to `sc.pp.log1p(adata.X)`
logged = sc.pp.log1p(adata, out=True)

out makes the singledispatch use some functions have (sc.pp.log1p(adata.X)) unnecessary, so we might deprecate that, what do you think?

Example for how this improves code:

-logged = sc.pp.log1p(adata_obs, copy=True)
-sc.pp.highly_variable_genes(logged)
-adata_obs = adata_obs[:, logged.var["highly_variable"]]
+sc.pp.log1p(adata_obs, out="layers.log1p")
+sc.pp.highly_variable_genes(adata_obs, use="layers.log1p")
+adata_obs = adata_obs[:, adata_obs.var["highly_variable"]]

TODO:

  • use needs to be consistently documented
  • use_raw needs deprecating
  • scale still writes var["mean"]/["std"] under out=True, should maybe gain a return_stats parameter or so
  • regress_out copies a backed array before rejecting it

@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.35361% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.93%. Comparing base (5f2accb) to head (418caa0).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/scanpy/get/get.py 81.52% 17 Missing ⚠️
src/scanpy/preprocessing/_simple.py 83.33% 5 Missing ⚠️
src/scanpy/preprocessing/_scale.py 85.00% 3 Missing ⚠️
src/scanpy/tools/_rank_genes_groups.py 77.77% 2 Missing ⚠️
src/scanpy/tools/_score_genes.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main    #4359       +/-   ##
=========================================
+ Coverage      0   81.93%   +81.93%     
=========================================
  Files         0      134      +134     
  Lines         0    12795    +12795     
=========================================
+ Hits          0    10483    +10483     
- Misses        0     2312     +2312     
Flag Coverage Δ
hatch-test.low-vers 78.48% <71.48%> (?)
hatch-test.pre 81.78% <88.97%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/scanpy/_docs.py 100.00% <100.00%> (ø)
src/scanpy/_utils/__init__.py 76.68% <100.00%> (ø)
src/scanpy/experimental/_docs.py 100.00% <100.00%> (ø)
...c/scanpy/experimental/pp/_highly_variable_genes.py 100.00% <100.00%> (ø)
src/scanpy/experimental/pp/_normalization.py 100.00% <100.00%> (ø)
src/scanpy/get/__init__.py 100.00% <ø> (ø)
src/scanpy/get/_aggregated.py 96.37% <100.00%> (ø)
src/scanpy/metrics/_gearys_c.py 100.00% <100.00%> (ø)
src/scanpy/metrics/_morans_i.py 100.00% <100.00%> (ø)
src/scanpy/neighbors/__init__.py 81.93% <100.00%> (ø)
... and 18 more

... and 106 files with indirect coverage changes

@flying-sheep flying-sheep added this to the 1.13.0 milestone Sep 11, 2026
@grst

grst commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

functions operating on a main matrix get use and out – default for out is to operate in place, out=None means returning the matrix instead of writing it into AnnData.

If we want to discourage use of inplace we could also make the out param explicit:

  • not specified -> fails
  • out=False -> inplace
  • out=True -> return

@flying-sheep

Copy link
Copy Markdown
Member Author

Yeah, I think bool instead of None|Default is better. I thought that users probably don’t need to pass a dynamic value (since it completely changes how the function works), but out=False for inplace and out=True for return is just more intuitive.

@grst

grst commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Or sentinels, e.g. A.INPLACE | A.RETURN? But I guess the boolean is nicer to type.

@flying-sheep

Copy link
Copy Markdown
Member Author

Yeah, an enum would of course be clearest, but I think the bool is intuitive enough, don’t you think?

I currently left the default to be False, i.e. “overwrite what use points to”, but it doesn’t need to stay that, we could:

  • force people to specify out=... as you suggest, or
  • use function-dependent conventions like e.g. the defaults for log1p could be use="layers.counts" and out="layers.logged" or something.

@grst

grst commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

use function-dependent conventions like e.g. the defaults for log1p could be use="layers.counts" and out="layers.logged" or something.

I like that, this would establish some conventions

@flying-sheep

Copy link
Copy Markdown
Member Author

Yeah! I’ll meet with Ilan today about this, and we’ll probably hash something out. I intentionally left it out of this PR so far since the design is still changing, so I don’t want to muddle the discussion with bikeshedding about which conventional names to settle on.

@flying-sheep

Copy link
Copy Markdown
Member Author

Plan:

  • remove key_added
  • for multi-out, add global lookup and validation table for each function’s slots, sth. like sc.OUTS["pp.neighbors"] == dict(distances=A.obsm["distances"], connectivities=A.obsm["connectivities"])
  • either:
    • add prefix parameter to all use/out functions, as mutually exclusive replacement to out
    • allow to use a class like sc.Prefix("foo") for out, e.g. sc.pp.neighbors(..., use=sc.Prefix("foo"))

@flying-sheep

Copy link
Copy Markdown
Member Author

OK, multi-out is blocked on not having .uns accessors then: scverse/anndata#2657

Storing accessors for e.g. neighbors in anndata is scverse/anndata#1979 (and the follow-up of storing accessors)

This branch has not been deployed

No deployments
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.

anndata.acc replacement for functions taking obsm, layer, … Remove inplace/copy

2 participants