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

Misuse of rstrip in make_variants_long_table.py script #277

Closed
fmaguire opened this issue Feb 15, 2022 · 3 comments
Closed

Misuse of rstrip in make_variants_long_table.py script #277

fmaguire opened this issue Feb 15, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@fmaguire
Copy link

Hi folks.

There is a problem with the use of rstrip to remove file suffixes at https://github.com/nf-core/viralrecon/blob/master/bin/make_variants_long_table.py#L47 that can cause issues (and a workflow failure) for some sample names.

rstrip doesn't remove a specified substring from the end of string.
It actually removes any characters (and any number of them) regardless of order in the substring from the end of string.
rstrip only stops when it encounters a character in the string not in the specified substring.
e.g.,

"abaaacccca....ccaac".rstrip("ac.")
>>> "ab"

This means sample names containing characters in the suffix being removed get mangled:

filename='XXXX_P0_meta.oilcans.saponin.pangolin.csv'
suffix=".pangolin.csv"
print(filename.rstrip(suffix))
>>>XXXX_P0_met

Replacing rstrip with removesuffix should fix this in python >=3.9 (alternatively replace should be fine although a simple regex would be better!).

@drpatelh drpatelh added the bug Something isn't working label Feb 15, 2022
@drpatelh drpatelh changed the title Bug related to misuse of rstrip Misuse of rstrip in make_variants_long_table.py script Feb 15, 2022
drpatelh added a commit to drpatelh/nf-core-viralrecon that referenced this issue Feb 15, 2022
drpatelh added a commit that referenced this issue Feb 15, 2022
@drpatelh
Copy link
Member

Hi @fmaguire ! Thanks for reporting! Created a patch release for v2.3.1 that I will get out today. A single word change turned into a bit of a 🐰 🕳️ and I have updated a bunch of other stuff too.

@fmaguire
Copy link
Author

Hah, cheers!

I figured it might (e.g., which python version is used etc) despite being a seeming trivial fix, that is why I didn't just submit a quick PR.

@drpatelh
Copy link
Member

Release done 🎉 https://github.com/nf-core/viralrecon/releases/tag/2.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants