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

support for samtools > 1.2 #49

Merged
merged 3 commits into from
Mar 8, 2016

Conversation

gedankenstuecke
Copy link
Contributor

When running circlator with samtools 1.3 the mapping step failed due to the error below. This is because starting from samtools 1.3 you have to specify the -o option for the output-file, otherwise it will not run. You can already start using this parameter from version 1.2 on.

As an easy workaround I now just checked for the version of samtools before running the sort-step and either run it with or without the -o flag.

The following command failed with exit code 1
samtools sort -@ 1 -m 500M 01.mapreads.bam.tmp.unsorted.bam 01.mapreads

The output was:

[bam_sort] Use -T PREFIX / -o FILE to specify temporary and final output files
Usage: samtools sort [options...] [in.bam]
Options:
  -l INT     Set compression level, from 0 (uncompressed) to 9 (best)
  -m INT     Set maximum memory per thread; suffix K/M/G recognized [768M]
  -n         Sort by read name
  -o FILE    Write final output to FILE rather than standard output
  -T PREFIX  Write temporary files to PREFIX.nnnn.bam
  -@, --threads INT
             Set number of sorting and compression threads [1]
      --input-fmt-option OPT[=VAL]
               Specify a single input file format option in the form
               of OPTION or OPTION=VALUE
  -O, --output-fmt FORMAT[,OPT[=VAL]]...
               Specify output format (SAM, BAM, CRAM)
      --output-fmt-option OPT[=VAL]
               Specify a single output file format option in the form
               of OPTION or OPTION=VALUE
      --reference FILE
               Reference sequence FASTA FILE [null]

@martinghunt martinghunt self-assigned this Mar 4, 2016
@martinghunt
Copy link
Collaborator

Thanks for reporting this, and the fix. Code looks fine to me.

Travis CI is complaining when trying to install pysam, but that's unrelated to your changes. I'll need to fix that before merging...

@gedankenstuecke
Copy link
Contributor Author

Thanks, glad you can use the changes! On my end all tests succeeded, but then I did see that there are none so far for the mapping step.

Right now I'm running it on a small test-set on my end to see whether the changes run through fine.

@martinghunt
Copy link
Collaborator

Hmm, you're right. I will also complete the unit tests for the mapping. I left placeholders for them but haven't filled them in yet.

@gedankenstuecke
Copy link
Contributor Author

Great, I'll keep you posted on how it works on my end until then. 😃

@gedankenstuecke
Copy link
Contributor Author

Okay, that worked well on my end 👍 If I can be of any help for writing the tests, let me know :-)

@martinghunt
Copy link
Collaborator

Great, thanks for the update. I'll merge your PR, then do the Travis fix and add unit tests, and make a new release...

martinghunt added a commit that referenced this pull request Mar 8, 2016
@martinghunt martinghunt merged commit b29ec01 into sanger-pathogens:master Mar 8, 2016
@gedankenstuecke
Copy link
Contributor Author

Great, thanks so much!

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