support for conversion of cellranger filtered h5 to h5mu - #1170
Conversation
dorien-er
changed the title
| if input_data.get("filtered_counts"): | ||
| mudata_per_sample = process_counts_filtered( | ||
| input_data["filtered_counts"], | ||
| ) | ||
| else: | ||
| mudata_per_sample = process_counts( | ||
| input_data["count"], | ||
| input_data["multiplexing_analysis"], | ||
| input_data["metrics_summary"], | ||
| ) |
There was a problem hiding this comment.
Hmm, when I put the mudata construction here I already had the feeling that it would not be the best idea...Should we make it separate from from the processing of the auxiliary modalities?
So in the main function construct the mudata's with separate functions and pass those on into the get_modalities function?
And have a different function for detecting the input? Like detect_count_matrices and detect_modality_input? That would also remove some complexity from the input detection?
I'm not sure, what do you think?
There was a problem hiding this comment.
Makes sense, there's a proposal in the latest commit :-)
| input_data = gather_input_data(cellranger_multi_dir) | ||
| result = get_modalities(input_data) | ||
| validate_input_directory(cellranger_multi_dir) | ||
| count_input = detect_count_matrices(cellranger_multi_dir) |
There was a problem hiding this comment.
This is a step in the good direction! One thing that might make this code easier to interpret is to replace the _list_library_types with a function get_library_type that takes the name of the library type as input (e.g. get_library_type("counts")). I noticed that after _list_library_types is used, the result is almost immediately filtered. In order to avoid overhead when doing the same calculation multiple times, @functools.cache can be used. This way, detect_modality_input would no longer need count_dir as a parameter; it can just also call _list_library_types("count")?
After this is done, I think in the main function, the mudata's can be constructed first (with only count matrices), then add the other modality information on top of them? Only the counts and multiplexing information are needed to handle the count matrices.
2 participants
Changelog
... Describe your changes ...
Issue ticket number and link
Closes #xxxx (Replace xxxx with the GitHub issue number)
Checklist before requesting a review
I have performed a self-review of my code
Conforms to the Contributor's guide
Check the correct box. Does this PR contain:
Proposed changes are described in the CHANGELOG.md
CI tests succeed!