Rm fix remaining bugs

[robmaz]

I think the recommended way to use it will be to provide the ReadTools arguments directly in the new format, in which case this get_trimming_args function will not be used. You only provided this function to translate the old trimming args to the new format, and we did not have an old maxArgs option. If we want to support old command lines, which I think was the idea, we need to provide a default here.

R.

2018-08-20 14:45 GMT+02:00 Daniel Gómez-Sánchez notifications@github.com:

@magicDGS commented on this pull request.

In lib/perl5/site_perl/ReadToolsUtils.pm https://github.com/robmaz/distmap/pull/107#discussion_r211247640:

@@ -64,7 +64,7 @@ Output: =cut sub get_trimmingargs { my ($qualThreshold, $minLength, $discardRemainingNs, $noTrimQuality, $no5ptrim) = @;

• my $trim_args = " --readFilter ReadLengthReadFilter --minReadLength $minLength";
• my $trim_args = " --readFilter ReadLengthReadFilter --minReadLength $minLength --maxReadLength 1000";

@robmaz https://github.com/robmaz - actually I think that the maximum read lenght should be an opt-in for the new version of ReadTools. The main problem is that the read-length filter is inherit from GATK, and it is true that it should be required (at least one of min/max read length). They decided to have the max as the required one, and the min set by default to 1 - which makes sense, but not for the case of the distmap pipeline...

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/robmaz/distmap/pull/107#pullrequestreview-147626158, or mute the thread https://github.com/notifications/unsubscribe-auth/Ad_FfJG_oF6Tqd7dqe931W-OQYkErTpDks5uSq9ugaJpZM4WD60T .

[magicDGS]

In that case, default should be larger - 1000 might filter some stuff when uploading, for example, long reads. Thus, I recommend a much larger number than 1000.