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

bam2fasta package to simplify sourmash compute #768

Merged
merged 6 commits into from Nov 2, 2019

Conversation

@pranathivemuri
Copy link
Contributor

pranathivemuri commented Oct 31, 2019

  • Removes bam2fasta conversion bits from sourmash compute

  • adds bam2fasta dependency

  • names tests separated out when input-is-10x

  • adds bam2fasta pip dependency and removes pathos, pysam

  • Is it mergeable?

  • make test Did it pass the tests?

  • make coverage Is the new code covered?

  • Did it change the command-line interface? Only additions are allowed
    without a major version increment. Changing file formats also requires a
    major version number increment.

  • Was a spellchecker run on the source code and documentation after
    changes were made?

@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 31, 2019

Codecov Report

Merging #768 into master will increase coverage by 1.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #768      +/-   ##
==========================================
+ Coverage   88.42%   89.48%   +1.05%     
==========================================
  Files          30       29       -1     
  Lines        4786     4554     -232     
  Branches       46       46              
==========================================
- Hits         4232     4075     -157     
+ Misses        551      476      -75     
  Partials        3        3
Impacted Files Coverage Δ
sourmash/command_compute.py 96.55% <100%> (+20.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94ae59c...fc08bfa. Read the comment docs.

@pranathivemuri

This comment has been minimized.

Copy link
Contributor Author

pranathivemuri commented Oct 31, 2019

@olgabot @ctb @luizirber Please review when you are free! Here is the pip package I released for bam2fasta conversion - https://pypi.org/project/bam2fasta/

Copy link
Member

luizirber left a comment

Two changes requested:

sourmash/command_compute.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Oct 31, 2019

We also need to get bam2fasta into bioconda before releasing a new version of sourmash. This might be harder than expected, because there is already a bam2fastx recipe, and bioconda doesn't like very similarly named packages (see bioconda/bioconda-recipes#18263 (comment)).

@pranathivemuri

This comment has been minimized.

Copy link
Contributor Author

pranathivemuri commented Oct 31, 2019

@luizirber do you have a config file/recipe file you used to release sourmash on bioconda. I have released the pip bam2fasta packages, thanks to release.md instructions in sourmash/docs. I saw the bioconda's autobump feature as one of the steps, but it didn't have much details how to create a recipe there.

@pranathivemuri

This comment has been minimized.

Copy link
Contributor Author

pranathivemuri commented Oct 31, 2019

@luizirber addressed your comments, thanks!

@luizirber

This comment has been minimized.

Copy link
Member

luizirber commented Oct 31, 2019

@luizirber addressed your comments, thanks!

Wow, that was fast! =]

As a rule of thumb, if you need to change a test it's probably a breaking change for the API. That's one reason why we never remove tests, and avoid changing them as much as possible...

do you have a config file/recipe file you used to release sourmash on bioconda. I have released the pip bam2fasta packages, thanks to release.md instructions in sourmash/docs. I saw the bioconda's autobump feature as one of the steps, but it didn't have much details how to create a recipe there.

I usually start from a previous recipe that was recently updated. The official documentation is https://bioconda.github.io/contributor/index.html, and the linked https://github.com/bioconda/bioconda-recipes/blob/master/recipes/pyfaidx/meta.yaml is a pretty good start. It's also a good idea to add yourself as a maintainer (see https://github.com/bioconda/bioconda-recipes/blob/faf2f3ecbc2093cec7237974683f8bcdb64d1e47/recipes/salmon/meta.yaml#L41 for an example)

@pranathivemuri

This comment has been minimized.

Copy link
Contributor Author

pranathivemuri commented Nov 1, 2019

@luizirber bam2fasta can be downloaded via bioconda now, just tried it! Thanks for the comments/leads on how to start the bioconda package. Thanks to your directions and the release.md in sourmash repo, I can put pip and bioconda packages in my skills bucket now! :D

Please review this PR when you are free!

@luizirber luizirber merged commit 92c8bf9 into dib-lab:master Nov 2, 2019
7 checks passed
7 checks passed
LGTM analysis: C/C++ No code changes detected
Details
LGTM analysis: JavaScript No code changes detected
Details
LGTM analysis: Python No new or fixed alerts
Details
Travis CI - Pull Request Build Passed
Details
codecov/patch 100% of diff hit (target 88.42%)
Details
codecov/project 89.48% (+1.05%) compared to 94ae59c
Details
netlify/sourmash-docs/deploy-preview Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.