katholt / srst2

Short Read Sequence Typing for Bacterial Pathogens
Other
125 stars 65 forks source link

Couple of fixes for #48

Closed SionBayliss closed 8 years ago

SionBayliss commented 9 years ago

Dear Kat,

Firstly, thank you for providing such an excellent program. I have been using SRST2 on a few of our more unusual bugs. I encountered a couple of problems that I have fixed. As they were minor fixes, but big problems, I thought you would like to know.

1/ When using the --report_all_consensus. I was getting a bug which originated from line 764 where the output file contained a directory path. It was appending the directory path to the output file and generating an error. I fixed it by replacing

outpileup = allele_name + "." + all_pileup_file

with

outpileup = allele_name + "." + os.path.basename(all_pileup_file)

2/ I was getting a problem where the pileup file was empty but there was clearly coverage when viewing the bam file and sufficient reads were mapping. This seems to be a problem with the default options for samtool mpileup. I fixed this by adding the -A (map anomalous reads) option to line 622. While it may not be good practice to always consider unpaired reads, all of my reads were considered unpaired (all though they were a pe file). It maybe worth either making this the default option or providing an option to consider anomalous reads without having to dig into the code.

All the best, Sion Bayliss

katholt commented 8 years ago

Thanks Sion. Number 1 is now resolved under issue #40. Number 2 I have fixed by adding an optional parameter --samtools_args to facilitate passing extra arguments to samtools mpileup. Both will be in v0.1.6 release shortly.