Define R API functions in separate script #103
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
bmeluch
changed the title
| # results as a single dataframe (can be nested). It uses the next_page_token | ||
| # key in each page of results to retrieve the following page. | ||
|
|
||
| get_next_results <- function(collection, filter_text, max_page_size, fields) { |
There was a problem hiding this comment.
You might consider renaming this function to something like get_all_results because that's actually what it's doing (not just getting the next ones).
| # size, and a list of the fields to be retrieved. | ||
| # It returns the metadata as a dataframe. | ||
|
|
||
| get_first_page_results <- function(collection, filter, max_page_size, fields) { |
There was a problem hiding this comment.
Consider adding roxygen annotations for each function that describes the parameters. https://cran.r-project.org/web/packages/roxygen2/vignettes/rd.html . This helps with auto completion and is generally the best practice for documenting functions in R.
| get_results_by_id <- function(collection, match_id_field, id_list, fields, max_id = 50) { | ||
| # collection: the name of the collection to query | ||
| # match_id_field: the field in the new collection to match to the id_list | ||
| # id_list: a list of ids to filter on |
There was a problem hiding this comment.
Are these actually R lists or are they vectors? I think vectors?
| output[[i]] = get_next_results( | ||
| collection = collection, | ||
| filter = filter, | ||
| max_page_size = max_id*3, #assumes that there are no more than 3 records per query |
There was a problem hiding this comment.
Remove this *3 and associated comment, they aren't true. Consider adding the max_page_size parameter to this function.
| @@ -14,85 +14,21 @@ | |||
| "cell_type": "code", | |||
There was a problem hiding this comment.
| @@ -14,85 +14,21 @@ | |||
| "cell_type": "code", | |||
There was a problem hiding this comment.
| @@ -14,85 +14,21 @@ | |||
| "cell_type": "code", | |||
There was a problem hiding this comment.
| @@ -14,85 +14,21 @@ | |||
| "cell_type": "code", | |||
There was a problem hiding this comment.
| @@ -14,85 +14,21 @@ | |||
| "cell_type": "code", | |||
There was a problem hiding this comment.
|
@bmeluch - the sourcing for the functions doesn't work in google colab. Here's a link to the NOM notebook in this branch in google colab - it'd be good to have this in the PR description because this is something we should be testing during each new notebook. When I try to run the first cell, it errors out with the new sourcing. I'm pretty sure @samobermiller had figured out a workaround for the python functions to make this work, hopefully the same will work for R? NOM notebook in colab: Taxonomy notebook in colab: |
This PR pulls the R API function definitions out of notebooks and defines them all in a single R script, so we aren't accidentally defining the same function with different tweaks in different notebooks.
It modifies two existing notebooks and adds a script
utility_functions.R(Eventually this may become an R package, but R scripts seem to play nicer than R packages when embedded in a repo.)
All Submissions:
Notebook Fix Submissions: