-
Notifications
You must be signed in to change notification settings - Fork 0
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 STRait Razor STR loci config settings #68
Conversation
if len(match_list) == 0: | ||
lus, sec, ter = 0, 0, 0 | ||
elif len(match_list) == 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While running some test data, I ran into an error when lusSTR encountered a short sequence from the DYS448 locus. While this likely won't be encountered often, it prevents lusSTR from breaking.
if locus == "D12S391" and kit == "powerseq": | ||
if "." in str(marker.canonical): | ||
check_sr += 1 | ||
if check_sr > 10: | ||
msg = ( | ||
"Multiple microvariants identified at D12 locus. " | ||
"Please check STRait Razor version!!" | ||
) | ||
print(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new SR config for D12 has a change of 9 bases, so using an older STRait Razor config file results in the assignment of almost all micro variants (e.g. 13.1
, 12.1
, 9.1
, etc.). While these can occur when using the correct SR version, a large number of micro variants indicates a potential problem. This will just output a message directing the user to look into it (and identifies a potential fix).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want to print this to stderr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want it to stop lusSTR though- just warn the user. Will printing to stderr do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. stderr is the conventional output stream for warning and error messages, stdout is for...output. 😀 Both are printed to the terminal, but can be redirected separately (e.g. mycmd > out.txt 2> err.txt
). Printing warning messages to stderr is a best practice that makes sure that the user still sees them even if they redirect output to a file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super critical. You can handle this in #67—I didn't want it to hold up this PR.
This is ready for review, but the CI keeps failing. I thought it would be the Snakemake version but changing it didn't seem to help. Any ideas? Google wasn't helpful. |
setup.py
Outdated
"snakemake>=7.22.0", | ||
"snakemake>=7.32.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Snakemake version 8.0 was released on December 20, so you were on the right track there. What's needed is not an updated minimum version, but an explicit maximum version. You probably want something like snakemake>=7.22,<8.0
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup! that worked!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
if locus == "D12S391" and kit == "powerseq": | ||
if "." in str(marker.canonical): | ||
check_sr += 1 | ||
if check_sr > 10: | ||
msg = ( | ||
"Multiple microvariants identified at D12 locus. " | ||
"Please check STRait Razor version!!" | ||
) | ||
print(msg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably want to print this to stderr.
The most recent versions of STRait Razor config files (specifically
PowerSeqv2.31.config
) havesignificantsome changes, affecting multiple STR loci. The following loci need adjustments in thestr_markers.json
file in order to correctly filter the STR sequences to the correct region:PowerSeq:
D12S391
D18S51
TPOX
Y-GATA-H4DYS643DYS458DYS456DYS448DYS439DYS393DYS391DYS390DYS389IINo longer have DYS389IDYS385DYS19ForenSeqD13S317D18S51D19S433D1S1656D21S11D5S818D7S820Penta DvWAY-GATA-H4DYS643DYS522DYS461 (not currently present)DYS448DYS439DYS391DYS390DYS389IIDYS385DYS19DXS7423DXS10135DXS10103update: the changes were not as significant as I initially thought! When I looked more into it, only a few of the aSTR PowerSeq loci changed.