nf-core / rnaseq

RNA sequencing analysis pipeline using STAR, RSEM, HISAT2 or Salmon with gene/isoform counts and extensive quality control.
https://nf-co.re/rnaseq
MIT License
924 stars 709 forks source link

Add umicollapse as an alternative to umi-tools #1369

Open siddharthab opened 2 months ago

siddharthab commented 2 months ago

Checklist:

siddharthab commented 2 months ago

I checked the output of UMICollapse, and it definitely does not follow the assumptions stated in prepare_for_rsem.py. So PREPARE_FOR_RSEM will be needed for UMICollapse as long as it is needed for umi-tools.

siddharthab commented 3 weeks ago

@MatthiasZepper all remaining tasks are now done. Please take another look.

MatthiasZepper commented 3 weeks ago

@MatthiasZepper all remaining tasks are now done. Please take another look.

Unfortunately, the UMICollapse module has not yet been upgraded...

MatthiasZepper commented 2 weeks ago

@siddharthab: The module is finally upgraded, so you can replace the local path with the up-to-date module.

siddharthab commented 2 weeks ago

@siddharthab: The module is finally upgraded, so you can replace the local path with the up-to-date module.

Thanks. Can you check if everything looks good now? I just ran nf-core modules update umicollapse, but do not know if I need to do more.

siddharthab commented 2 weeks ago

Can we uppercase the new process aliases please?

@pinin4fjords, done.

pinin4fjords commented 1 week ago

@nf-core-bot fix linting

pinin4fjords commented 1 week ago

Sorry, good in principle now for me (maybe some factoring out we can do later). Just need to get tests to run + pass.

MatthiasZepper commented 1 week ago

Sorry, good in principle now for me (maybe some factoring out we can do later). Just need to get tests to run + pass.

Well, it appears the issue is on my end. Will fix asap! Sorry!

The UMICollapse tests are failing, because they try importing the BWA-Mem module, which isn’t included in this pipeline. As obviously everything is available in the modules directory, the tests pass in that context.

Considering that I had copied the tests from umitools dedup due to my limited experience with nf-test when creating the UMICollapse module, I was puzzled about why the umitools-tests work in here for the pipeline.

Turns out that I had coicidentially copied them as a template during a very brief period on March 4 before Maxime updated them again, which explains the discrepancy...

Long story short, I will have to fix the UMICollapse test in the modules directory to be self-reliant.

MatthiasZepper commented 1 day ago

Just to give a brief update: I have finally found some time to update the tests. Therefore, I hope the module tests will soon be updated in the repo and subsequently this PR also be merged successfully!