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

Bumping pysam to 0.13 *core*, without customization #4497

Merged
merged 6 commits into from Nov 13, 2017

Conversation

Projects
None yet
6 participants
@dannon
Member

dannon commented Aug 25, 2017

Htslib and pysam now support flexible index names/paths, so we can finally kill https://github.com/dannon/pysam/tree/flexindex_84. This depends on galaxyproject/starforge#143 being fixed and merged (which I am working on).

@galaxybot galaxybot added the triage label Aug 25, 2017

@galaxybot galaxybot added this to the 17.09 milestone Aug 25, 2017

@dannon

This comment has been minimized.

Member

dannon commented Sep 8, 2017

(rebased to fix conflicts)

@@ -8,8 +8,7 @@
usage: %prog in_file out_file
"""
import optparse
from pysam import ctabix

This comment has been minimized.

@nsoranzo

nsoranzo Sep 8, 2017

Member

I think you should keep the blank line, optparse is part of the standard library, while pysam is not.

This comment has been minimized.

@dannon

dannon Sep 8, 2017

Member

Yep, agreed, I'll tweak it in a sec.

@dannon

This comment has been minimized.

Member

dannon commented Sep 8, 2017

Wheel still under construction, there was a library inclusion fail there. Will retest when it's re-published.

@galaxyproject galaxyproject deleted a comment from natefoo Sep 8, 2017

@galaxyproject galaxyproject deleted a comment from dannon Sep 8, 2017

natefoo added some commits Sep 11, 2017

Explicitly include PyPI as an --extra-index-url to pip so we can install
from PyPI even when a package exists (but the correct version or
architecture does not exist) on wheels.galaxyproject.org. Previously,
PyPI was only checked after wheels.galaxyproject.org because pypiserver
performs an automatic redirect to PyPI for any packages that do not
exist in its index.

@dannon dannon added status/review and removed status/WIP labels Sep 11, 2017

keep_original=True, index_filename=out_fname)
pysam.tabix_index(filename=index_fname, seq_col=(options.chrom_col - 1),
start_col=(options.start_col - 1), end_col=(options.end_col - 1),
keep_original=True, index_filename=out_fname)

This comment has been minimized.

@nsoranzo

nsoranzo Sep 12, 2017

Member

It looks like thay tabix_index in https://github.com/pysam-developers/pysam/blob/master/pysam/libctabix.pyx still doesn't support keep_original and index_filename parameters.

This comment has been minimized.

@mvdbeek

mvdbeek Sep 12, 2017

Member

That's unfortunate, because it looks like it's almost there: https://github.com/pysam-developers/pysam/blob/master/pysam/libchtslib.pxd#L1355.

I'll try to PR this.

This comment has been minimized.

@mvdbeek

mvdbeek Sep 12, 2017

Member

I see there are also some other enhancements in @dannon's branch, like detecting gzip compression instead of relying on the file ending, which would would probably need to include as well. Have we ever tried contributing this back ?

This comment has been minimized.

@mvdbeek

mvdbeek Sep 12, 2017

Member

let's see how this goes: pysam-developers/pysam#537

This comment has been minimized.

@dannon

dannon Sep 12, 2017

Member

Good catch, I guess I shouldn't have assumed both index methods had been updated :/

If you want, I can PR the gzip detection stuff.

Back when I wrote this, it would have required changes to htslib and pysam, and I never got around to doing it.

This comment has been minimized.

@mvdbeek

mvdbeek Sep 12, 2017

Member

If you want, I can PR the gzip detection stuff.

That would be great. There's also the keep_original setting, which I think we need as well.

@natefoo

This comment has been minimized.

Member

natefoo commented Sep 12, 2017

macOS and x32 Linux wheels are on wheels.galaxyproject.org via galaxyproject/starforge#161.

Although it sounds like we'll have to update again...

@dannon

This comment has been minimized.

Member

dannon commented Sep 12, 2017

We'll need to wait for pysam-developers/pysam#537, (merge and then release), and then we can proceed here. Changing milestone to 18.01.

@mvdbeek

This comment has been minimized.

Member

mvdbeek commented Nov 4, 2017

Alright, we got a new release https://pypi.python.org/pypi/pysam/0.13!

@dannon

This comment has been minimized.

Member

dannon commented Nov 4, 2017

@mvdbeek Nice! Last time the wheel building was a bit of a pain, so that may take a bit of effort, but I'll go ahead and update this branch tomorrow and test against 0.13

@nsoranzo

This comment has been minimized.

Member

nsoranzo commented Nov 5, 2017

Can we use the wheels on PyPI? https://pypi.python.org/pypi/pysam

@dannon

This comment has been minimized.

Member

dannon commented Nov 5, 2017

@nsoranzo Yes, that's what we were going to do when we were attempting to go to 0.12.01. We still have to build and host the osx wheel, though.

@dannon

This comment has been minimized.

Member

dannon commented Nov 6, 2017

Non-pypi'd wheels PR open at galaxyproject/starforge#178, though I was able to run this branch on my osx box (automatically fetched source from pypi and built fine). I think the selenium test failure is unrelated (passes fine on my local machine).

@dannon dannon changed the title from Bumping pysam to 0.12 *core*, without customization to Bumping pysam to 0.13 *core*, without customization Nov 13, 2017

@mvdbeek

Awesome, thanks a lot @dannon !

@mvdbeek mvdbeek merged commit a2c164a into galaxyproject:dev Nov 13, 2017

6 of 7 checks passed

selenium test Build finished. 98 tests run, 0 skipped, 1 failed.
Details
api test Build finished. 309 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 162 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 57 tests run, 0 skipped, 0 failed.
Details
lgtm analysis: JavaScript No alert changes
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@natefoo

This comment has been minimized.

Member

natefoo commented Nov 13, 2017

🎺 🎺 🎺 🎺

@dannon dannon referenced this pull request Dec 5, 2017

Closed

Tabix indexing fixes. #5131

@martenson martenson removed the status/WIP label Jul 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment