-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement harmonypy component & make changes to concat #54
Conversation
Integration CI @ https://github.com/openpipelines-bio/openpipeline/actions/runs/2969733996 |
Please merge #55 first! |
* add harmony component * Fix missing imports. * Fix typo in changelog * harmonize integrate/harmony and integrate/harmonypy Co-authored-by: Robrecht Cannoodt <[email protected]>
…the anndata/mudata
src/integrate/concat/script.py
Outdated
if obs_key_sample_name in sample.obs_keys(): | ||
logger.info(f'Column .obs["{obs_key_sample_name}"] already exists in sample "{sample_id}". Overriding the value for this column.') | ||
samples.obs = sample.obs.drop(obs_key_sample_name, axis=1) | ||
for _, modality in sample.mod.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use for modality in sample.mod.values()
src/integrate/concat/script.py
Outdated
@@ -189,7 +186,7 @@ def set_conflicts(concatenated_data: mu.MuData, | |||
getattr(mod, conflict_matrix_name)[conflict_name] = conflict_data.sort_index() | |||
return concatenated_data | |||
|
|||
def concatenate_modalities(modalities: dict[str, anndata.AnnData], | |||
def concatenate_modalities(sample_ids: tuple[str], modalities: dict[str, list[anndata.AnnData]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I made a mistake here. A general Iterable[anndata.Anndata]
is better here
I compared this implementation with the implementation from #55
❯ hyperfine --warmup 1 './target/docker/integrate/harmony/harmony -i mms_with_batch.h5mu -o ./test.h5mu' Benchmark 1: ./target/docker/integrate/harmony/harmony -i mms_with_batch.h5mu -o ./test.h5mu Time (mean ± σ): 25.057 s ± 1.703 s [User: 0.177 s, System: 0.128 s] Range (min … max): 23.303 s … 29.393 s 10 runs
❯ hyperfine --warmup 1 './target/docker/integrate/harmonypy/harmonypy -i mms_with_batch.h5mu -o ./test.h5mu' Benchmark 1: ./target/docker/integrate/harmonypy/harmonypy -i mms_with_batch.h5mu -o ./test.h5mu Time (mean ± σ): 6.851 s ± 0.399 s [User: 0.167 s, System: 0.114 s] Range (min … max): 6.364 s … 7.703 s 10 runs
docker stats for
harmony
docker stats for
harmonypy