Skip to content
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

Fix HISAT2 strandedness parameter #1470

Merged
merged 12 commits into from
Sep 26, 2017
17 changes: 14 additions & 3 deletions tools/hisat2/hisat2.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0"?>
<tool id="hisat2" name="HISAT2" version="2.0.5.2" profile="17.01">
<tool id="hisat2" name="HISAT2" version="2.0.5.3" profile="17.01">
<description>A fast and sensitive alignment program</description>
<macros>
<import>hisat2_macros.xml</import>
Expand Down Expand Up @@ -464,7 +464,7 @@
<output name="output_unaligned_reads_l" file="test_unaligned_reads.fasta" />
<output name="output_unaligned_reads_r" file="test_unaligned_reads.fasta" />
</test>
<test><!-- Ensure fastqsanger.gz works -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave the comment here and below, any specific reason to remove them?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason to remove so I put those comments back. I only removed them because they were getting in the way of me commenting out tests while doing my own testing. I just forgot to put them back. I put them back after your review here but then I removed them again for more testing and then forgot to put them back again :( They're back again now.

<test>
<param name="input_format_selector" value="fastq" />
<param name="paired_selector" value="paired" />
<param name="collection_selector" value="files" />
Expand All @@ -480,7 +480,7 @@
<param name="no_discordant" value="True" />
<output name="output_alignments" file="hisat_output_3.bam" ftype="bam" lines_diff="2" />
</test>
<test><!-- Ensure fastqsanger.bz2 works -->
<test>
<param name="input_format_selector" value="fastq" />
<param name="paired_selector" value="paired" />
<param name="collection_selector" value="files" />
Expand All @@ -496,6 +496,17 @@
<param name="no_discordant" value="True" />
<output name="output_alignments" file="hisat_output_3.bam" ftype="bam" lines_diff="2" />
</test>
<test>
<param name="input_format_selector" value="fastq" />
<param name="paired_selector" value="paired" />
<param name="collection_selector" value="files" />
<param name="reference_genome_source" value="history" />
<param name="history_item" ftype="fasta" value="phiX.fa" />
<param name="forward" ftype="fastqsanger" value="hisat_input_1_forward.fastq" />
<param name="reverse" ftype="fastqsanger" value="hisat_input_1_reverse.fastq" />
<param name="rna_strandedness" value="F" />
Copy link
Member

@nsoranzo nsoranzo Sep 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/rna_strandedness/rna_strandness/
This may actual cause the output bam to be different, I imagine)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this highlighted that my test really was terrible! That strandness parameter was never used because of the typo so my test was checking against a incorrect bam! The lines diff check doesn't catch that but checking the header would, so if there is ever a way in the future to check a bam header that could be cool to help make tests like this one more robust.

<output name="output_alignments" file="hisat_output_4.bam" ftype="bam" lines_diff="2" />
</test>
</tests>
<help><![CDATA[
Introduction
Expand Down
10 changes: 9 additions & 1 deletion tools/hisat2/hisat2_macros.xml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,15 @@
<token name="@strandedness_parameters@">
#if str($spliced_options.spliced_options_selector) == "advanced":
#if str($spliced_options.rna_strandness).strip() != '':
--rna-strandness $spliced_options.rna_strandness
#if str($input_format.paired.paired_selector) == 'paired':
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you use a tool for checking indentation? my eyes don't seem to be very reliable :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mblue9 Unfortunately I'm not aware of tool to check indentation for the Cheetah code inside XML, so basically it's my eyes (on GitHub) or searching for 4 spaces in the text editor.

Copy link
Member

@shiltemann shiltemann Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mblue9 no worries, @nsoranzo just has special eagle-eye superpowers the rest of us don't possess ;) 🦅 👀

(but most editors have a setting to show whitespace characters and/or indentation guides, which helps for me)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks for the tips!

#if str($spliced_options.rna_strandness) == 'F':
--rna-strandness FR
#else:
--rna-strandness RF
#end if
#else:
--rna-strandness $spliced_options.rna_strandness
#end if
#end if
#end if
</token>
Expand Down
Binary file added tools/hisat2/test-data/hisat_output_4.bam
Binary file not shown.