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

Update cli.py #793

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update cli.py #793

wants to merge 1 commit into from

Conversation

MaWeffm
Copy link

@MaWeffm MaWeffm commented Jul 9, 2024

From the documentation, it seems that in paired-end mode, lower-case parameters are applied to the forward read, and upper-case parameters are applied to the reverse read. To keep this scheme consistent, this small change allows to shorten only one of the reads using either -l for the forward read, or -L for the reverse read.

Add -l and -L options to work separately on paired-end reads.
@marcelm
Copy link
Owner

marcelm commented Jul 17, 2024

Hi, thank you for the suggestion, and I agree this behavior would be more consistent with what the other options do. However, I needed to implement it this way for backwards compatibility: When -l is provided by itself, the original behavior was to trim both reads to the same length, and that needs to remain.

BTW, the -q/-Q options (quality trimming) have the same problem (if -q is given, it sets the trimming threshold for both R1 and R2).

The way to only shorten R1 and not R2 would be to specify a value for R2 that exceeds the read length, like -l 10 -L 100000. Maybe one option is to make it possible to write a special value for -L to disable it like -l 10 -L disable.

Brainstorming a bit: Your original suggestion in #788 of not adding an -L option and using colons to separate the two lengths would actually work better here: -l 10 does the same as before, -l 10:15 specifies different lengths, and -l :15 and -l 10: would be used to only shorten R1 or R2. For consistency, this syntax could be introduced for -U and -Q as well. Using it for -A, -B, -G probably doesn’t make much sense, so some inconsistency would remain.

@MaWeffm
Copy link
Author

MaWeffm commented Jul 18, 2024

Thank you for your kind response! I see the need of backwards compatibility, please feel free to decline my pull request.

Currently, I already follow your suggestion to provide large numbers as a workaround to avoid shortening R1 or R2. In terms of consistency, I like the idea using the colon to separate the lengths, also for the -U and -Q parameters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants