search

deweylab / RSEM

RSEM: accurate quantification of gene and isoform expression from RNA-Seq data
http://deweylab.biostat.wisc.edu/rsem/
GNU General Public License v3.0
408 stars 118 forks source link

Convert RSEM to use the HTSlib API instead of the legacy samtools API #19

Closed jmarshall closed 8 years ago

jmarshall commented 8 years ago

This pull request converts the RSEM source code to use HTSlib for its BAM-reading needs rather than the old samtools 0.1.x API. Since the old API is unmaintained (and has been for several years), we would recommend converting to the new API wholesale rather than continuing with the old API (cf @outpaddling's samtools/samtools#531).

This compiles against just htslib, but I have not tested it or even tried to run it. So there would need to be a bedding-in period, especially of the fresh code in 5852e1ef73648606e02eb92c0bc49c0ccab121b4.

I realise you have some local changes in _samtools-1.3/bamsort.c (ccec130769207255411170f316587b5160ff6ae0). We are still considering how best to deal with your samtools/samtools#520; most likely mainline samtools sort -n will gain an ability to keep mates together.

bli25wisc commented 8 years ago

Hi John,

Thanks for these commits. I have merged them.

bli25wisc commented 8 years ago

Hi John,

So in the future, [bs]am.h files will not be maintained?

Thanks, Bo

jmarshall commented 8 years ago

Thanks for merging this, Bo! Let me know if it causes any problems.

Samtools itself no longer uses [bs]am.h, they're just there to aid in compiling old third-party code that uses them. So this means they don't get any new functionality, e.g., you can probably open CRAM files via the old API's samopen() (though I haven't tested this for a while), but you can't adjust any of the CRAM settings that you can adjust when using HTSlib directly.

So they've been pretty much unmaintained for a while. In fact, I'm getting a bit of pressure to delete them or to make them print out "bam.h is deprecated -- use the HTSlib API instead", and the latter will probably happen in samtools 1.4.

outpaddling commented 8 years ago

Hi John,

I vote for the deprecated warning message. I support HPC clusters and have ported a number of apps that depend on samtools. Many of them have been slow to keep up. I think some still require 0.1.19. I'm sure it's just because the developers are busy and don't have time to look for important changes. I think a warning message would lead to easier deployment of a lot of software in the near future.

Thanks again to all of you for your work on this.

Jason