Skip to content

chore: clarify that no I/o is needed for add_adatas#182

Open
ilan-gold wants to merge 3 commits into
mainfrom
ig/clarify_docs
Open

chore: clarify that no I/o is needed for add_adatas#182
ilan-gold wants to merge 3 commits into
mainfrom
ig/clarify_docs

Conversation

@ilan-gold
Copy link
Copy Markdown
Collaborator

@ilan-gold ilan-gold commented Mar 29, 2026

This just popped into my head but people don't really need to create stores all at once in pipelining.

Something like:

for path in range(my_adata_paths):
    adata = anndata.read_h5ad(path)
    preprocessed_adata = some_preprocessing(adata)
    def load_adata_by_path(path):
        return preprocessed_adata
    collection.add_adatas([path], load_adata=load_adata_by_path, ...)
my_one_big_adata = ad.concat([ad.read_zarr(p) for p in paths])
my_preprocessed_big_adata = some_preprocessing(my_one_big_adata)
def load_adata_by_path(path):
    return my_preprocessed_big_adata
collection.add_adatas(["preprocessed_by_felix"], load_adata=load_adata_by_path, ...)

is probably a pretty common use-case, especially if people don't know about anndata.concat lazy.

@ilan-gold ilan-gold requested a review from felix0097 March 29, 2026 09:24
@ilan-gold ilan-gold added the skip-gpu-ci Whether gpu ci should be skipped label Mar 29, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.20%. Comparing base (022e8c8) to head (3367314).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #182      +/-   ##
==========================================
- Coverage   93.19%   91.20%   -1.99%     
==========================================
  Files          14       14              
  Lines        1058     1058              
==========================================
- Hits          986      965      -21     
- Misses         72       93      +21     
Files with missing lines Coverage Δ
src/annbatch/io.py 90.06% <ø> (ø)

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ilan-gold
Copy link
Copy Markdown
Collaborator Author

ping @felix0097

@felix0097
Copy link
Copy Markdown
Collaborator

A summary of the discussion points I had earlier:

The use case with the some_preprocessing function can be moved into the load_adata function. This is less convoluted and better fits our current architecture:

def load_adata(path):
    adata = ad.experimental.load_lazy(path)
    adata = some_preprocessing(adata)
    
    return adata

this can also be expanded to custom preprocessing logic per path argument, e.g like this

load_fct_dict = {path: custom_load_fct, ....}

def load_adata(path):
    adata = ad.experimental.load_lazy(path)
    adata =load_fct_dict[path](adata)
    
    return adata

In general, if people subset the var or obs space in the pre-processing, your size estimates for the dataset shard size and zarr shard size will be off. We could add check when writing the data to disk to see if we're off by a factor of e.g >2. And then yield a user warning, saying they could adjust the dataset_size and shard_size arguments.

Also, as our size estimation logic depends on actually being able to access the on disk data. We should add checks that supplying non existing paths raises an error, e.g. this would be disallowed:

collection.add_adatas(["preprocessed_by_felix"], load_adata=load_adata_by_path, ...)

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

Labels

skip-gpu-ci Whether gpu ci should be skipped

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants