qiime2 / q2-dada2

QIIME 2 plugin wrapping DADA2
BSD 3-Clause "New" or "Revised" License
19 stars 36 forks source link

Update Q2-DADA2 to the new (1.6.0) R package release version #78

Closed benjjneb closed 6 years ago

benjjneb commented 6 years ago

After some trial and error, I realized that you must have both the bioconda AND the conda-forge channels in order to install this recipe, as bioconda doesn't have some of the dependencies built for r3.4.1. But the below does work in a local conda environment:

conda config --add channels bioconda
conda config --add channels conda-forge
conda install bioconductor-dada2=1.6.0=r3.4.1_0

And the following confirms package version 1.6.0:

R -e "library(dada2);packageVersion('dada2')"
thermokarst commented 6 years ago

Thanks @benjjneb! For this to make it into the 2017.12 release cycle it will need to be merged by 12/19. Let us know if you need any help, otherwise, just drop the 'DO-NOT-MERGE' tag when you are ready for one of us to take a look. Thanks so much!

thermokarst commented 6 years ago

Also, I am available to help out with this PR, just ping me wherever is convenient for you, @benjjneb!

benjjneb commented 6 years ago

I think this is OK to merge. I tried to recreate the failling test on my machine, but it doesn't fail, and I suspect it may have something to do with the differences in the R environment being used in the tests (i.e. R 3.3.2 and older package versions) and the newer 3.4.1 versions. But once it gets into the CI build environments, it starts to get a little beyond me.

thermokarst commented 6 years ago

Thanks @benjjneb --- the travis instances run the latest q2 environment file, so when we bump dependency versions we usually wind up with a bit of a chicken-and-egg problem --- travis can't test the latest version, because the latest version hasn't been merged (it is a vicious cycle).

If this PR is ready for review, one of us will pull it down and test locally, since travis can't really help us right now. Thanks!

benjjneb commented 6 years ago

Yes its ready to be reviewed.

thermokarst commented 6 years ago

Merging even though travis is failing, because of the reasons mentioned above. Thanks!