pirate 1.0.2 #17798
pirate 1.0.2 #17798
Conversation
|
Package(s) built on CircleCI are ready for inspection:
You may also use
Docker image(s) built:
|
tseemann
force-pushed
the
pirate-1.0.2
branch
2 times, most recently
from
6d669e7
to
f00791d
Compare
|
tseemann
force-pushed
the
pirate-1.0.2
branch
2 times, most recently
from
897e484
to
9e31928
Compare
|
Gracias! |
|
@npavlovikj @tseemann why was this PR merged? |
|
Sorry about the confusion @tseemann - I saw your comment about the failing, and just approved the PR so you can merge it after those tests pass (since the recipe looks ok and the failing is not because of the recipe itself)... Looks like Bjorn was faster and merged the recipe in the meantime. |
|
@npavlovikj Ah ok. Well there is an issue of the |
|
@tseemann , I would suggest you to use the extended base: I just tried the mulled tests locally with that change, and I believe the tests finished fine: |
|
@npavlovikj thank you! for enlightening me and for checking it works. this is good news for @SionBayliss |
|
@tseemann @npavlovikj Thanks for your help on this. I just checked the package and it installs correctly and runs on an example dataset. The plotting script doesn't work due to a newer version of some of the packages not installing their own dependencies. @npavlovikj should I prepare a pull request with the fixed version of the r-packages? I have checked them and they function correctly. |
|
I couldn't check out this branch, so opened a new PR with adding the extended-base, #17831. |
|
@npavlovikj - the R dependencies work fine when fixed to specific versions. Would you recommend this as part of the main package? Alternatively, the R dependencies could be removed as I have included these as optional dependencies to the main package in the README. In either case the R dependencies are: I have made a new release (v1.0.3) which contains some very minor changes. |
|
@SionBayliss , what error the pinning solves? The update of the recipe is really easy - beside humans, there is a bot that checks for new releases and opens new PR (actually, looks like there is already a new PR for pirate, #17948). This Bioconda recipe is defined as "noarch:generic", which means it will work on both Linux and OSX platforms. I don't know how the one on your channel is built, but I would suggest you to use |
|
@npavlovikj I wasn't aware that bioconda was so efficient :). If that is the case could we remove the R dependencies from the recipe altogether? The warning |
|
@npavlovikj How would you recommend that I go about correcting this? I have made a local branch of bioconda-recipes and removed the R dependencies from PIRATE. Should I submit that as a pull request? Also, it looks like conda has only kept the most recently tagged version on PIRATE. Was that manually done or will it continue to remove older versions of PIRATE when newly tagged version are released? Also, thanks for your help with this :) |
|
@SionBayliss , sorry for my delay - I got busy with work. If you already have a ready branch, please go ahead and open PR in bioconda, and feel free to tag me for review. In case you are not sure for the steps, please check https://bioconda.github.io/contributor/workflow.html. Regarding the older versions - you can find all versions in the channel itself, https://anaconda.org/bioconda/pirate/files. It is only in the recipe directory that the newest release is kept. |
|
@npavlovikj I know how that feels :). I did a little debugging of the Rscripts and there are some dependency issues with the most recent versions of the R libs. One of the libraries, I think phangorn, has a dependency on phytools which is not installing correctly due to a library that isn't being correctly identified, even when it is present. I am open to including the optional dependencies if they are fixed or if they will install correctly via conda. I would currently prefer to remove them as I don't want users to be given the wrong impression that PIRATE includes a broken script when the dependencies for the script can be easily installed via the README. I am happy to update the recipe or script with later releases. I have posted a pull request but I am not a member of the project so I couldn't tag it ( #18128 ). |
|
Good job with your first PR @SionBayliss - looks like Devon already merged your request :) |
Bioconda requires reviews prior to merging pull-requests (PRs). To facilitate this, once your PR is passing tests and ready to be merged, please add the
please review & mergelabel so other members of the bioconda community can have a look at your PR and either make suggestions or merge it. Note that if you are not already a member of the bioconda project (meaning that you can't add this label), please ping@bioconda/coreso that your PR can be reviewed and merged (please note if you'd like to be added to the bioconda project). Please see #15332 for more details.