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

Avoid using private HTSlib headers and functions #53

Merged
merged 1 commit into from
Dec 10, 2020
Merged

Avoid using private HTSlib headers and functions #53

merged 1 commit into from
Dec 10, 2020

Conversation

jmarshall
Copy link
Contributor

Use a public HTSlib API function to set the CRAM reference file rather than an internal CRAM-only function that is private to HTSlib.

Happily there is only one instance of a private HTSlib function being used. Fixes #21.

Only headers in htslib/*.h are part of HTSlib's public API; cram/*.h
in particular are not. Fortunately SeqLib only uses one function from
cram/cram_io.h, and that is easily replaced with a public equivalent.

Instead use `hts_set_fai_filename()`, which strdups the string given to
it itself; `cram_set_option(fp->fp.cram, CRAM_OPT_REFERENCE, ...)` would
also be an option, but it's better to avoid `fp->fp.cram` as poking into
an htsFile's insides is also not supported by HTSlib.
@jmarshall
Copy link
Contributor Author

I see there has been some recent activity in SeqLib. @walaj It would be greatly appreciated if you would consider and merge this one too.

@pjotrp
Copy link

pjotrp commented Dec 10, 2020

Great 👍

@walaj walaj merged commit 5e3a7bf into walaj:master Dec 10, 2020
@walaj
Copy link
Owner

walaj commented Dec 10, 2020

This is completely on me for missing this for so long, saw the comment on Twitter. Thank you for this fix.

@tillea
Copy link

tillea commented Dec 10, 2020

Thanks a lot for fixing. Would you mind tagging a new release?

@jmarshall jmarshall deleted the no-htslib-privates branch December 10, 2020 19:31
@jmarshall
Copy link
Contributor Author

Thanks for merging it. As @tillea suggests, it would be ideal if this appeared in a new release in due course. But certainly having this commit on master does the trick for projects that have SeqLib as a git submodule.

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.

Could SeqLib and htslib work out what a public cram api should be?
4 participants