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

Add r-base as a dependency #16398

Merged
merged 7 commits into from
Aug 5, 2019
Merged

Add r-base as a dependency #16398

merged 7 commits into from
Aug 5, 2019

Conversation

rhpvorderman
Copy link
Contributor

r-base is needed for picard CollectMultipleMetrics, CollectInsertSizeMetrics etc. (basically anything with metrics).

  • 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).

r-base is needed for picard CollectMultipleMetrics, CollectInsertSizeMetrics etc. (basically anything with metrics).
@rhpvorderman rhpvorderman added the please review & merge set to ask for merge label Jul 16, 2019
@bgruening
Copy link
Member

#3859 :(

@rhpvorderman
Copy link
Contributor Author

#3859 :(

@bgruening. I posted my opinion on the issue.

@rhpvorderman rhpvorderman removed the please review & merge set to ask for merge label Jul 16, 2019
@rhpvorderman
Copy link
Contributor Author

@bgruening , given that there was no great backlash on the issue, I think that nobody strongly cares about this. Shall we go ahead and merge this?

@bgruening
Copy link
Member

@dpryan79 any opinion?

@dpryan79
Copy link
Contributor

dpryan79 commented Aug 2, 2019 via email

@cbrueffer
Copy link
Member

I tend to agree that things should work out of the box, although I have to admit I shared Devon's unawareness regarding the R dependency. Given the bloat having a picard-lite recipe and pointing to that in the picard install message would be good.

@biocondabot
Copy link
Contributor

biocondabot bot commented Aug 5, 2019

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

Arch Package Repodata
noarch picard-2.20.4-1.tar.bz2 repodata.json
noarch picard-slim-2.20.4-0.tar.bz2 repodata.json

You may also use conda to install these:

  • For packages in noarch:
    conda install -c https://67568-42372094-gh.circle-artifacts.com/0/tmp/artifacts/packages <package name>
    

Docker image(s) built:

Package Tag Install with docker
picard-slim 2.20.4--0
showcurl "https://67568-42372094-gh.circle-artifacts.com/0/tmp/artifacts/images/picard-slim%3A2.20.4--0.tar.gz" | gzip -dc | docker load
picard 2.20.4--1
showcurl "https://67568-42372094-gh.circle-artifacts.com/0/tmp/artifacts/images/picard%3A2.20.4--1.tar.gz" | gzip -dc | docker load

@rhpvorderman
Copy link
Contributor Author

rhpvorderman commented Aug 5, 2019

@bgruening @dpryan79 @cbrueffer
The previous iteration of picard has been renamed to picard-slim.
While copying the package I found that a bash wrapper script is used to run picard instead of a python wrapper script. So I removed the python dependency in both picard and picard-slim.

Is this compromise to your liking?

EDIT: Also updated the branch with master.

@dpryan79
Copy link
Contributor

dpryan79 commented Aug 5, 2019

👍 from me. @bgruening @cbrueffer thoughts?

@cbrueffer
Copy link
Member

Looks good; a post-install message in each of the recipes pointing to the other one would be nice ("A package without the R dependency is available as picard-slim or similar); see e.g. recipes/braker2/post-link.sh.

@bgruening
Copy link
Member

Fine with me ...

@rhpvorderman
Copy link
Contributor Author

rhpvorderman commented Aug 5, 2019

Looks good; a post-install message in each of the recipes pointing to the other one would be nice ("A package without the R dependency is available as picard-slim or similar); see e.g. recipes/braker2/post-link.sh.

@cbrueffer I do agree that users need to be notified. While working on the post-link script however, I felt that I had to write a really, really good message that would be worth it for the user to see every time they install picard. This is IMO not possible. It is a very annoying way to let users know.

I have added information about picard and picard-slim to the package descriptions, so that users can make an informed decision on which package to install based on the information on bioconda.github.io.

If these descriptions are okay I will merge the branch.

@cbrueffer
Copy link
Member

Thanks @rhpvorderman , I like the idea.

@cbrueffer cbrueffer merged commit 7410e38 into master Aug 5, 2019
@cbrueffer cbrueffer deleted the rhpvorderman-patch-1 branch August 5, 2019 12:30
kpalin pushed a commit to kpalin/bioconda-recipes that referenced this pull request Aug 7, 2019
* Add r-base as a dependency

r-base is needed for picard CollectMultipleMetrics, CollectInsertSizeMetrics etc. (basically anything with metrics).

* sanitize input

* add picard-slim package

* remove unnecessary python dependency

* better descriptions.
rhpvorderman added a commit to biowdl/BamMetrics that referenced this pull request Aug 12, 2019
rhpvorderman added a commit to biowdl/germline-DNA that referenced this pull request Aug 12, 2019
@alanhoyle
Copy link

Can this be added to the gatk4 as a dependency as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
please review & merge set to ask for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants