search

biobakery / biobakery_workflows

bioBakery workflows is a collection of workflows and tasks for executing common microbial community analyses using standardized, validated tools and parameters.
http://huttenhower.sph.harvard.edu/biobakery_workflows
Other
97 stars 33 forks source link

removed the rename code #19

Closed sagun98 closed 2 years ago

sagun98 commented 2 years ago

Issue linked with https://github.com/biobakery/hutlab/issues/5 Kneaddata v0.10.0, TRF was not generating the output file extension ".repeat.removed" which is failing the kneaddata task. I will sync up with Lauren in the chat as well.

ljmciver commented 2 years ago

Fantastic debugging @sagun98 ! Thank you. I just tested kneaddata to double check. Older versions of kneaddata had the "repeats.removed" as the final output file but it looks like now with the latest version it is just the output-prefix.

Awesome PR! Would you consider removing just a couple more lines.

  1. You could take out all references to the variable rename_final_output since that function no longer uses it. This will be 2 more changes.
  2. Would you remove the [args[3]] from the task (in the task string and also in the args list)?
  3. Can you also remove all places the variable kneaddata_output_repeats_removed_fastq is referenced? This will be 2 more changes.

With these additional changes it will just remove any other code that is no longer used which will make reading the function in the future easier.

Oh, one last thing. If you would update the documentation to note that the workflows require kneaddata v0.10.0+ it would help us with future debugging (since now the workflow works with the newer kneaddata but will not work with the older kneaddata).

Thanks again! Lauren

sagun98 commented 2 years ago

Thank you for the detailed code review @ljmciver. I have updated the pull request with the changes. Please let me know if anything else needs to be changed.

ljmciver commented 2 years ago

Looks great! Thanks @sagun98!