Skip to content
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

Add get_records() to API and df() to Registry #54

Merged
merged 8 commits into from
Oct 25, 2024
Merged

Conversation

lazappi
Copy link
Collaborator

@lazappi lazappi commented Oct 18, 2024

Adds a get_records() method to the InstanceAPI class which performs the API call to retrieve record summary information and a df() method to the Registry class which makes the required API calls to get the requested number of records and formats the returned information as a data.frame(). This tries to be semi-smart to limit the number of API calls.

Fixes #43

Checks:

  • CI runs
  • Update CHANGELOG

Comment on lines +94 to +98
# Hacky way of avoiding unneeded requests until there is an easy way
# to get the total number of records
if (isTRUE(attr(.data_list, "finished"))) {
return(.data_list)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a way to get the total number of records we can use a regular purrr::map instead

@lazappi lazappi requested a review from rcannood October 18, 2024 13:16
Copy link
Collaborator

@rcannood rcannood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Could you add a unit test for df()?

@lazappi lazappi requested a review from rcannood October 24, 2024 06:17
Copy link
Collaborator

@rcannood rcannood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lazappi lazappi merged commit 5d249c8 into main Oct 25, 2024
6 of 7 checks passed
@lazappi lazappi deleted the registry-to-data.frame branch October 25, 2024 06:26
lazappi added a commit that referenced this pull request Oct 25, 2024
…ables

* origin/main:
  Add progress bar (#58)
  Add get_records() to API and df() to Registry (#54)
lazappi added a commit that referenced this pull request Oct 25, 2024
…ggests

* origin/main:
  Remove link tables from print output (#55)
  Add progress bar (#58)
  Add get_records() to API and df() to Registry (#54)
lazappi added a commit that referenced this pull request Oct 28, 2024
* origin/main:
  Remove link tables from print output (#55)
  Add progress bar (#58)
  Add get_records() to API and df() to Registry (#54)
lazappi added a commit that referenced this pull request Oct 31, 2024
…-colours

* origin/main:
  Add Records container class (#59)
  Refactor documentation (#67)
  Fix macOS runner (#66)
  Add check_requires() function and update installation docs (#56)
  Remove link tables from print output (#55)
  Add progress bar (#58)
  Add get_records() to API and df() to Registry (#54)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add conversion to data.frame to Registry class
2 participants