feat: Allow using accessors in sc.get.aggregate#4199
Conversation
flying-sheep
commented
Jul 6, 2026
flying-sheep
commented
- Closes #
- Tests included or not required because:
- Release notes not necessary because:
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4199 +/- ##
==========================================
+ Coverage 79.91% 80.02% +0.11%
==========================================
Files 121 121
Lines 12949 13012 +63
==========================================
+ Hits 10348 10413 +65
+ Misses 2601 2599 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
flying-sheep
changed the title
sc.get.aggregate
| raise TypeError(msg) | ||
| data = adata[acc] | ||
| dim_df = pd.DataFrame({ | ||
| ref.idx if isinstance(ref.idx, str) else str(ref): adata[ref] |
There was a problem hiding this comment.
Idk how to do this better:
- only simplify
A.obs[k]to string? If so, should we allow to mix strings andAdRefs inby=[…]? - simplify unless there are duplicate
.idxs? - don’t stringify at all and use
AdRefsasdf.columns?
There was a problem hiding this comment.
Is the question here about the isinstance check? Like, what to do about people who pass in the index as a by column?
This line of code seems inoffensive other than I am not sure why ref.idx would be anything other than a str
There was a problem hiding this comment.
the question is about “we now accept AdRefs, what to use as dataframe columns when they’re used”
I am not sure why
ref.idxwould be anything other than astr
>>> type(ad.acc.A.obsm["y"][:, 0].idx)
intThere was a problem hiding this comment.
the question is about “we now accept AdRefs, what to use as dataframe columns when they’re used”
But these columns should really just be ref.idx since ad.acc.A.obsm["y"][:, 0] isn't a valid by unless I'm missing something.
There was a problem hiding this comment.
why would it not be?
import numpy as np
import scanpy as sc
from anndata import acc, AnnData
adata = AnnData(np.random.rand(3, 4), obsm=dict(thing=np.array(["x", "y", "y"])))
sc.get.aggregate(adata, acc.A.obsm["thing"][:, 0], ())
# AnnData object with n_obs × n_vars = 2 × 4
# obs: "A.obsm['thing'][:, 0]", 'n_obs_aggregated'There was a problem hiding this comment.
To clarify, if you leave it as-is, I think that's fine. If you revert to the old behavior, I think that's also perfectly reasonable (and would remove this question)
There was a problem hiding this comment.
I'm leaving towards what you have now, at least in terms of functionality. Being able to do one-versus-rest type aggregations, perhaps over a neighborhood graph, could be cool, if it's supported i.e., vec of 1s and 0s (neighbors and not neighbors). Not sure if people will really do this though, but just a thought.
There was a problem hiding this comment.
what do you think of the third option from my initial comment in this thread?
There was a problem hiding this comment.
Oh missed this
don’t stringify at all and use AdRefs as df.columns?
Is this serializable? What if someone tries to write their results?
There was a problem hiding this comment.
Hmm, the problem is that stringifying it introduces one more format that isn’t really supported. We have
- the actual
AdRefobject - the simple string version (supported via
A.resolve, e.g."obs.some_col". Can only represent a subset of possible refs) - the
repr(not supported as conversion format anywhere)
We could fix that by doing something like
- here, if passing a collection of
AdRefs, serialize torepralways (instead of the conditional.idxwe have now) - in
A.resolve, also support the repr format (if input.startswith('A.'): …)
then people can just use A.resolve to get back the input AdRefs from the obs column names.
Or we serialize/deserialize AdRefs in AnnData …
ilan-gold
left a comment
ilan-gold
left a comment
There was a problem hiding this comment.
I don't see any tests for the added behavior i.e., around by=A.obsm["foo"][:, 1]
| result = sc.get.aggregate(adata, by=A.obsm["thing"][:, 0], func=["sum", "mean"]) | ||
| expected = sc.get.aggregate(adata, by="blobs", func=["sum", "mean"]) |
There was a problem hiding this comment.
Does it make sense to check the names of the result i.e., result.obs_names?
There was a problem hiding this comment.
Also the obs of result should contain a certain column derived from by, right?
2 participants
