scverse / anndata

Annotated data.
http://anndata.readthedocs.io
BSD 3-Clause "New" or "Revised" License
577 stars 154 forks source link

concat errors with one dimensional arrays in obsm #652

Open ivirshup opened 2 years ago

ivirshup commented 2 years ago

Seen in comment by @Hrovatin in https://github.com/theislab/anndata/issues/501#issuecomment-979916818_

import anndata as ad, numpy as np

foo = ad.AnnData(np.ones((5, 20), obsm={"1d-array": np.ones(5)})

ad.concat([foo, foo])
Traceback ```pytb --------------------------------------------------------------------------- IndexError Traceback (most recent call last) /var/folders/bd/43q20k0n6z15tdfzxvd22r7c0000gn/T/ipykernel_14520/2434769132.py in ----> 1 ad.concat([a, a]) ~/github/anndata/anndata/_core/merge.py in concat(adatas, axis, join, merge, uns_merge, label, keys, index_unique, fill_value, pairwise) 848 [a.layers for a in adatas], axis=axis, reindexers=reindexers 849 ) --> 850 concat_mapping = inner_concat_aligned_mapping( 851 [getattr(a, f"{dim}m") for a in adatas], index=concat_indices 852 ) ~/github/anndata/anndata/_core/merge.py in inner_concat_aligned_mapping(mappings, reindexers, index, axis) 455 els = [m[k] for m in mappings] 456 if reindexers is None: --> 457 cur_reindexers = gen_inner_reindexers(els, new_index=index, axis=axis) 458 else: 459 cur_reindexers = reindexers ~/github/anndata/anndata/_core/merge.py in gen_inner_reindexers(els, new_index, axis) 476 reindexers = [Reindexer(df_indices(el), common_ind) for el in els] 477 else: --> 478 min_ind = min(el.shape[alt_axis] for el in els) 479 reindexers = [ 480 gen_reindexer(pd.RangeIndex(min_ind), pd.RangeIndex(el.shape[alt_axis])) ~/github/anndata/anndata/_core/merge.py in (.0) 476 reindexers = [Reindexer(df_indices(el), common_ind) for el in els] 477 else: --> 478 min_ind = min(el.shape[alt_axis] for el in els) 479 reindexers = [ 480 gen_reindexer(pd.RangeIndex(min_ind), pd.RangeIndex(el.shape[alt_axis])) IndexError: tuple index out of range ```

Off the top of my head, I can't think of a reason this working would be inconsistent with the anndata model. Generally I would expect people to put 1d arrays in obs though.

Might have implications for the concatenation of awkward arrays (#647)

Hrovatin commented 2 years ago

Yes, it was mistake from my side to put one-d array (forgot to reshape it). But I got no warning when I made adata and then just by chance figured out what was the reason.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. Please add a comment if you want to keep the issue open. Thank you for your contributions!

ilan-gold commented 5 months ago

@ivirshup @flying-sheep Should this work actually? Or should we start enforcing a multi-dimensional-ness for obsm? https://anndata.readthedocs.io/en/latest/generated/anndata.AnnData.obsm.html#anndata.AnnData.obsm makes it pretty clear that "Stores for each key a two or higher-dimensional" - does this mean that even something with shape [10, 1] should be disallowed?