-
Notifications
You must be signed in to change notification settings - Fork 24
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
docs: add README for Curation API notebooks #620
Conversation
Codecov Report
@@ Coverage Diff @@
## main #620 +/- ##
=======================================
Coverage 83.00% 83.00%
=======================================
Files 19 19
Lines 1700 1700
=======================================
Hits 1411 1411
Misses 289 289
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
notebooks/curation_api/README.md
Outdated
@@ -0,0 +1,25 @@ | |||
# Curation API notebooks |
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.
change to discovery and say formerly know as curation notebooks through out the doc
I would prefer that this README is not added at this time. The curators are already quite familiar with the API. I'm planning to ask Clever Canary to perform some work in this area later. |
@brianraymor we got questions recently from another user who was new to the notebooks, and this question has come up from time to time from others in DM as well, including CC. I am not seeing a downside to adding this very simple README which can be deleted if necessary in the future (when CC improve upon it?). |
I would appreciate having this readme added. It avoids me a few extra click to figure out which flavor of discovery API I actually need to jump into. Could we ask Clever Canary to update or remove as needed, if they plan on doing something better? |
notebooks/curation_api/README.md
Outdated
### Python notebooks: `python/` | ||
|
||
These notebooks are the most user-friendly; start here if you're unsure. All access token generation, url construction, and response | ||
handling is abstracted away from the user. Unitary interactions such as getting a Dataset, creating a Collection, etc., |
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.
creating a collection is a curator action, not a consumer action.
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.
Correct. Do you mean to say that, as such, it should not be used as an example? Only consumer actions should be referenced here?
|
||
These notebooks show the construction of http requests at a more granular level, and as such may be useful for technical | ||
users. | ||
|
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.
and as such may be useful for technical users.
This might be useful for someone who wants to port the REST API to another language or wants more control of the responses?
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.
or wants more control over the request, yes (for us, at the moment, and partners, in the case of hitting a Discover API on rdev...)
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.
changed to only reference consumer actions...
|
||
### R notebooks: `R/` | ||
|
||
These notebooks are the same as the "raw" Python notebooks mentioned immediately previous, but in R. |
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.
It seems like this directory should be named R_raw to be symmetrical?
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.
Renamed (see diff below)
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 can also move the entire notebooks/curation_api/
dir to notebooks/discover_api/
in a follow-up PR...
@brianraymor if you are comfortable giving your ✅ then we can merge this |
This PR has not seen any activity in the past 2 weeks; if no one comments or reviews it in the next 3 days, this PR will be closed. |
@brianraymor ping -- hoping to merge this |
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 would also recommend replacing the references to individual notebooks in https://api.cellxgene.cziscience.com/curation/ui/#/ with something like:
See the [README](<insert link>] for example notebooks in python and R.
|
||
### Python notebooks: `python/` | ||
|
||
These notebooks are the most user-friendly; start here if you're unsure. All access token generation, url construction, and response |
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.
access token generation is confusing for non-curation use which does not require access tokens?
This PR has not seen any activity in the past 2 weeks; if no one comments or reviews it in the next 3 days, this PR will be closed. |
This PR was closed because it has been inactive for 17 days, 3 days since being marked as stale. Please re-open if you still need this to be addressed. |
Initial README for now. Can be improved on in the future. Please comment if some aspect is glaringly wrong or a substantial omission has been made.