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

Replace surveys$id[i] with "SV_xxxxxxx1" to close #341 #360

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

JeffreyRStevens
Copy link
Member

This PR addresses #341 by replacing surveys$id[i] with "SV_xxxxxxx1" in all documentation. This seems like a more robust way to name the survey ID, since deleting surveys could change a particular survey's position in the list of surveys. Therefore, I made the replacement in all instances in the README, vignette, and examples but describe how to use surveys$id[i] in the vignette. But I'm happy to alter this, if desired.

@juliasilge
Copy link
Collaborator

Thank you so much for this PR, @JeffreyRStevens!

I don't want to change all these use cases like this based on other feedback and use cases for our docs, but we can definitely add it to the docs for fetch_survey() (not replace the current approach). For example, we could edit those exmaples to something like:

#' # Retrieve a list of surveys
#' surveys <- all_surveys()
#'
#' # Retrieve a single survey using your `surveys` dataframe:
#' my_survey <- fetch_survey(surveyID = surveys$id[6])
#'
#' # Retrieve a single survey if you know the survey ID as a string:
#' my_survey <- fetch_survey(surveyID = "SV_xxxxxxxxxxxxxxx")
#'
#' # Retrieve your survey specifying other args:
#' my_survey <- fetch_survey(
#'   surveyID = surveys$id[6],
#'   start_date = "2018-01-01",
#'   end_date = "2018-01-31",
#'   limit = 100,
#'   label = TRUE,
#'   unanswer_recode = 999,
#'   verbose = TRUE,
#'   # Manually override EndDate to be a character vector
#'   col_types = readr::cols(EndDate = readr::col_character())
#' )

Are you interested in updating this PR, or doing a separate one for this change?

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.

2 participants