Speedup sc.get.obs_df

[fidelram]

By using array slicing, this codes improves ~10 fold the speed of sc.get.obs_df().

%timeit sc.get.obs_df(adata, list(adata.var_names[:100]) + ['louvain'])

before:
40.6 ms ± 2.38 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)

after:
4.45 ms ± 228 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

[ivirshup]

Good idea! Could you show some of the benchmarks you got those time improvements from?

Could you do the same for var_df?

Also, I think might need a few additional tests. I initially used the .{dim}_vector functions since I know they return numpy vectors. Can this be tested against AnnData objects with sparse X, as well as backed objects?

[ivirshup]

About use_raw with sc.get.var_df: I didn't include this because the semantics differ significantly from sc.get.obs_df, as raw can have a different number of variables. I think it makes more sense for a user to call sc.get.var_df(adata.raw, ...), since it's much more explicit that adata.raw.var and adata.raw.varm will be used.

[fidelram]

I like the ideas of using adata.raw when raw data wants to be used in general. This is an elegant solution to a source of endless confusion.

Regarding var_df I can remove the use_raw. However, I consider that since this option is everywhere it should be here as well. I find odd that is not present. I don't see a problem that the size of the dataframe will be different that would be expected.

[ivirshup]

It's not just that the length is different, is that sc.get.obs_df(adata, ["col"], use_raw=x)["col"] is the same regardless of the value of x, but it's different for var_df. I think it's easier to build code around functions with more orthogonal arguments.

However, I consider that since this option is everywhere it should be here as well.

Could we add an example of sc.get.var_df(adata.raw, ...), leave out use_raw for now, and see if anyone complains?

I've been trying to leave out use_raw on functions where variable length matters anyways. For example: adata.var_vector.

[fidelram]

Ok, will remove that as soon as I can

[ivirshup]

Just had a thought, I don't think this will work if X is a backed dense array and obs_names isn't sorted. h5py.Datasets require that the indices be in order. This should probably get a test case.

An alternative is _get_obs_rep(adata[obs_names], ...).copy(), but this will have performance issues with raw.

[ivirshup]

Internally in anndata, I index in-order then reorder the array for h5py.Dataset objects.

[fidelram]

How can we test for this? I added a test to read a backed dataset from the included datasets. Why this works? or by chance are the indices ordered?

[ivirshup]

The indices look ordered, since you're getting them like this: list(adata.var_names[:10]).

I think list(adata.var_names[10::-1]) would be enough to make it fail. In AnnDatavalid indices in random order are generated with functions inanndata/tests/helpers.py`.

[ivirshup]

Will these be in the right order?

Won't the value from X be in whatever order the appear, while the values in obs_names are in the order they were passed?

[ivirshup]

This has made me realize some stuff needs to get fixed in anndata, mostly around raw. In future, I think this should look like adata[obs_names].to_df(layer=layer), but we're not quite there yet.

Here's what you can do to make backed mode work for now:

idxs = adata.obs_names.get_indexer(obs_names)
idxs_order = np.argsort(idxs_order)
matrix = X[idxs[idxs_order], :][np.argsort(idxs_order)]

In the next major release of anndata I'm thinking we should export some of the utilities that are used for giving all these indexing operations a consistent interface.

[fidelram]

Yes. That is why this should be safe for backed mode. Later in the code, before the result is returned the columns are reorder to match the keys order.

[fidelram]

The previous test failed but is not clear to me why, as it passes the local tests (anndata 0.7.5). It seems that on travis server, backed slicing requires integer indices and will not work with a boolean vector. I changed to sorted integers hoping that this will solve the issue.

[ivirshup]

My thinking on my change is that I would like all the code that handles backed mode to be cleanly separated. I think this should be handled more cleanly on the anndata side, and once that's been done it's easier to replace the backed mode specific code if it's all together.