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

Delete the "samtools import" subcommand. #1185

Merged
merged 1 commit into from
Feb 5, 2020

Conversation

jkbonfield
Copy link
Contributor

@jkbonfield jkbonfield commented Feb 3, 2020

This was deleted from the help / usage statements in 2009 (cc207d8)
between 0.1.5 and 0.1.6. Thus it has never been a documented command
since the official launch of Samtools.

Import was just a wrapper around view, but the wrapper broke when we
added PG headers. While a fix is trivial, this PR simply removes the
dead and untested code instead.

Fixes #1183 in as far as turning the segmentation fault into a handled
usage error instead.

This was deleted from the help / usage statements in 2009 (cc207d8)
between 0.1.5 and 0.1.6.  Thus it has never been a documented command
since the official launch of Samtools.

Import was just a wrapper around view, but the wrapper broke when we
added PG headers.  While a fix is trivial, this PR simply removes the
dead and untested code instead.

Fixes samtools#1183 in as far as turning the segmentation fault into a handled
usage error instead.
@daviesrob daviesrob merged commit 616f2c3 into samtools:develop Feb 5, 2020
@mr-c
Copy link

mr-c commented Feb 5, 2020

Could we be nice and show the new syntax to the user?

samtools import ex1.fa.fai ex1.sam.gz ex1.bam  

becomes

samtools view -bt ex1.fa.fai -o ex1.bam ex1.sam.gz

@dpryan79
Copy link

dpryan79 commented Feb 5, 2020

I'd suggest not being nice here, since any user doing this is likely doing so many other things in antiquated ways that they should rethink everything.

jmarshall added a commit to jmarshall/pysam that referenced this pull request Feb 5, 2020
The legacy import command was broken in Samtools 1.10 (hat tip @mr-c)
and has been removed from future Samtools (see samtools/samtools#1185).
Use the equivalent `samtools view` command instead.

Update 00README.txt to the current text in samtools/examples/00README.txt
that it was copied from, and delete the samtools command samples as they
are not relevant for pysam.
@jkbonfield
Copy link
Contributor Author

Exactly my thought @dpryan79. I wanted the user to think about what they're doing and look at the man page to see all the other features of view and to decide whether their usage is appropriate rather than suggest more cargo-cult cut paste non-thinking work. Let's face it, since import was supported there have been new file formats, new SAM flags (SUPPLEMENTARY, etc) and so on.

Also IMO this is all a storm in a teacup. The fact is there have been over 10,000 installs of samtools with the broken import command and not a single user reported it being broken. We got acknowledgement an automatic test harness breaking, but it would appear in day to day usage this command has dropped out of common usage.

jmarshall added a commit to jmarshall/pysam that referenced this pull request Feb 20, 2020
The legacy import command was broken in Samtools 1.10 (hat tip @mr-c)
and has been removed from future Samtools (see samtools/samtools#1185).
Use the equivalent `samtools view` command instead.

Update 00README.txt to the current text in samtools/examples/00README.txt
that it was copied from, and delete the samtools command samples as they
are not relevant for pysam.
jmarshall added a commit to jmarshall/pysam that referenced this pull request Mar 19, 2020
Remove samimport / `samtools import`, which has been removed from
upstream samtools (samtools/samtools#1185).
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.

1.10 segfaults on pysam tests but 1.9 doesn't
4 participants