Skip to content

Remove htslib/samtools/bcftools dependencies#13093

Merged
dpryan79 merged 5 commits intobioconda:masterfrom
mvdbeek:fix_pysam_assertion_error
Aug 15, 2019
Merged

Remove htslib/samtools/bcftools dependencies#13093
dpryan79 merged 5 commits intobioconda:masterfrom
mvdbeek:fix_pysam_assertion_error

Conversation

@mvdbeek
Copy link
Copy Markdown
Contributor

@mvdbeek mvdbeek commented Jan 15, 2019

This fixes pysam-developers/pysam#640
which is an issue with the bioconda build of pysam.
Removing those dependencies will make pysam use the bundled
subset of htslib.
A side-effect of this is that pysam and samtools/bcftools/htslib
versions are now independent and can be mixed at will.

  • I have read the guidelines for bioconda recipes.
  • This PR adds a new recipe.
  • AFAIK, this recipe is directly relevant to the biological sciences (otherwise, please submit to the more general purpose conda-forge channel).
  • This PR updates an existing recipe.
  • This PR does something else (explain below).

@mvdbeek mvdbeek force-pushed the fix_pysam_assertion_error branch 3 times, most recently from 2fbd896 to 0438d8c Compare January 15, 2019 15:58
@kyleabeauchamp
Copy link
Copy Markdown
Contributor

This is probably fine, although it may slow down the peformance of pysam because the HTSLib built on Bioconda uses a faster ZLIB implementation (libdeflate) and I'm not sure if the vanilla pysam build will grab it. It also means people can have different versions of pysam and HTSLib installed in an env, which is probably good but could be confusing in cases.

@kyleabeauchamp
Copy link
Copy Markdown
Contributor

Looks like the problem is discuss upstream as well: samtools/htslib#813

@mvdbeek
Copy link
Copy Markdown
Contributor Author

mvdbeek commented Jan 15, 2019

HTSLib built on Bioconda uses a faster ZLIB implementation (libdeflate) and I'm not sure if the vanilla pysam build will grab it.

I think it does, it's unfortunate that we don't get the full build log, but you can see libdeflate being used in the failed previous build: https://circleci.com/gh/bioconda/bioconda-recipes/40770?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

@mvdbeek
Copy link
Copy Markdown
Contributor Author

mvdbeek commented Jan 18, 2019

So given that this is bugfix it'd be great if we can merge this. If you're concerned about not having samtools/bcftools/htslib I could add them back to the run dependencies.

mvdbeek added 2 commits June 13, 2019 11:02
This fixes pysam-developers/pysam#640
which is an issue with the bioconda build of pysam.
Removing those dependencies will make pysam use the bundled
subset of htslib.
A side-effect of this is that pysam and samtools/bcftools/htslib
versions are now independent and can no be mixed at will.
@mvdbeek mvdbeek force-pushed the fix_pysam_assertion_error branch from 0438d8c to d1c3a95 Compare June 13, 2019 10:52
@biocondabot
Copy link
Copy Markdown
Contributor

biocondabot Bot commented Jun 13, 2019

Package(s) built on CircleCI are ready for inspection:

Arch Package Repodata
linux-64 pysam-0.15.3-py27hda2845c_1.tar.bz2 repodata.json
linux-64 pysam-0.15.3-py36hda2845c_1.tar.bz2 repodata.json
linux-64 pysam-0.15.3-py37hda2845c_1.tar.bz2 repodata.json
osx-64 pysam-0.15.3-py27h726f235_1.tar.bz2 repodata.json
osx-64 pysam-0.15.3-py36h726f235_1.tar.bz2 repodata.json
osx-64 pysam-0.15.3-py37h726f235_1.tar.bz2 repodata.json

You may also use conda to install these:

  • For packages in linux-64:
    conda install -c https://69419-42372094-gh.circle-artifacts.com/0/tmp/artifacts/packages <package name>
    
  • For packages in osx-64:
    conda install -c https://69420-42372094-gh.circle-artifacts.com/0/tmp/artifacts/packages <package name>
    

Docker image(s) built:

Package Tag Install with docker
pysam 0.15.3--py27hda2845c_1
showcurl "https://69419-42372094-gh.circle-artifacts.com/0/tmp/artifacts/images/pysam%3A0.15.3--py27hda2845c_1.tar.gz" | gzip -dc | docker load
pysam 0.15.3--py36hda2845c_1
showcurl "https://69419-42372094-gh.circle-artifacts.com/0/tmp/artifacts/images/pysam%3A0.15.3--py36hda2845c_1.tar.gz" | gzip -dc | docker load
pysam 0.15.3--py37hda2845c_1
showcurl "https://69419-42372094-gh.circle-artifacts.com/0/tmp/artifacts/images/pysam%3A0.15.3--py37hda2845c_1.tar.gz" | gzip -dc | docker load

@bgruening
Copy link
Copy Markdown
Member

@mvdbeek is this still needed? Want to update the PR?

@mvdbeek
Copy link
Copy Markdown
Contributor Author

mvdbeek commented Aug 3, 2019

Not for the bug, which should be fixed, but I don't think it's reasonable to ship samtools with pysam -- the wheels don't come with samtools either.

@bgruening
Copy link
Copy Markdown
Member

Then please merge it with master. @kyleabeauchamp ok?

@kyleabeauchamp
Copy link
Copy Markdown
Contributor

Fine with me, but ping @AndreasHeger to confirm,

@AndreasHeger
Copy link
Copy Markdown
Contributor

Yes, removing samtools/bcftools should be fine - they are test requirements only.

I don't fully understand the htslib removal but if you are confident it will work I have no objections.

@dpryan79
Copy link
Copy Markdown
Contributor

If I didn't bork anything while handling the merge conflict then please merge :)

@kyleabeauchamp
Copy link
Copy Markdown
Contributor

Seems good still, as long as the build is green

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.

Setting VariantRecord pos or stop raises error

5 participants