-
Notifications
You must be signed in to change notification settings - Fork 200
Add support for CRAN and Bioconductor #1477
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
Conversation
Use the Posit Public Package Manager as an API endpoint to query for package version information TODO: Add Bioconductor support Refs: google#1476 Signed-off-by: Tyler <[email protected]>
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, thanks for working on this!
Just wanted to point out one thing:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good.
I just have a few questions:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM - I'm to approve/merge this when the lints are fixed (as long as the tests pass).
Also, I have a few small comments:
Thanks again for your work!
/gcbrun |
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good to go now - thanks!
I'm going to go ahead and merge this so we can get importing on our staging site.
Perfect, thanks for the patience @michaelkedar @andrewpollock great feedback, and feel free to reach out if there's anything else you need from me. |
def test_next_version(self): | ||
"""Test next_version.""" | ||
ecosystem = ecosystems.get('Bioconductor') | ||
self.assertEqual('1.18.0', ecosystem.next_version('a4', '1.16.0')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tylfin this is starting to fail, but I'm not seeing why?
$ GET https://packagemanager.posit.co/__api__/repos/4/packages/a4?bioc_version=1.16.0
{"code":2,"error":"No sources found for Bioconductor 1.16.0","payload":null}
$ GET https://packagemanager.posit.co/__api__/repos/4/packages/a4?bioc_version=1.18.0
{"code":2,"error":"No sources found for Bioconductor 1.18.0","payload":null}
Yet https://bioconductor.org/packages/release/bioc/html/a4.html suggests the package still exists?
Also:
$ GET https://packagemanager.posit.co/__api__/repos/4/packages/a4?bioc_version=1.50.1
{"code":2,"error":"No sources found for Bioconductor 1.50.1","payload":null}
$ GET https://packagemanager.posit.co/__api__/repos/4/packages/a4?bioc_version=1.50.0
{"code":2,"error":"No sources found for Bioconductor 1.50.0","payload":null}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I was driving that wrong, here's what's happening:
$ GET https://packagemanager.posit.co/__api__/repos/4/packages/a4?bioc_version=3.19
{"code":4,"error":"Package 'a4' not found","payload":null}
It seems to work with all of the earlier versions than 3.19, based on a spot check with the bounds of bioc_versions
This pull request adds support for CRAN and Bioconductor from the R advisory database such that they will populate on the osv.dev site, included:
Refs: #1476