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

Fix supertibble label #179

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Fix supertibble label #179

merged 2 commits into from
Mar 12, 2024

Conversation

ezraporter
Copy link
Collaborator

@ezraporter ezraporter commented Mar 12, 2024

Description

This PR fixes a bug that was causing one of our error messages to print incorrectly. Since it required learning a bit about custom print methods I also added a customization to the header when our supertibbles print. Let me know if you think we should keep it.

Proposed Changes

  • Added a vec_ptype_abbr.redcap_supertbl() method so that objects of the supertibble type get abbreviated to suprtbl in cli messages.
    • This is following advice from the vctrs vignette which advises keeping labels to 6 characters because these labels also appear when objects are elements of a tibble or list
  • Added a tbl_sum.redcap_supertbl() method so that printed supertibbles include a header with the object type and number of instruments
    • Check out this vignette if you're interested in other customizations we could add!
  • Added rerunning the cli examples to utility/refresh.R

Error Message Fix

Old

missing_col_supertbl <- tibble(redcap_data = list()) %>%
  as_supertbl()
make_labelled(missing_col_supertbl)
#> Error in `make_labelled()`:
#> ✖ You've supplied `<rdcp_spr[,1]>` for `supertbl` which is not a valid
#>   value
#> ! Must contain `supertbl$redcap_metadata`
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

New

#> Error in `make_labelled()`:
#> ✖ You've supplied `<suprtbl[,1]>` for `supertbl` which is not a valid
#>   value
#> ! Must contain `supertbl$redcap_metadata`
#> ℹ `supertbl` must be a REDCapTidieR supertibble, generated using
#>   `read_redcap()`

Custom Header

Old

read_redcap(Sys.getenv("REDCAP_URI"), Sys.getenv("REDCAPTIDIER_CLASSIC_API")) %>%
  suppressWarnings()
#> # A tibble: 9 × 9
#>   redcap_form_name       redcap_form_label redcap_data redcap_metadata structure
#>   <chr>                  <chr>             <list>      <list>          <chr>    
#> 1 nonrepeated            Nonrepeated       <tibble>    <tibble>        nonrepea…
#> 2 nonrepeated2           Nonrepeated2      <tibble>    <tibble>        nonrepea…
#> 3 repeated               Repeated          <tibble>    <tibble>        repeating
#> 4 data_field_types       Data Field Types  <tibble>    <tibble>        nonrepea…
#> 5 text_input_validation… Text Input Valid… <tibble>    <tibble>        nonrepea…
#> 6 api_no_access          API No Access     <tibble>    <tibble>        nonrepea…
#> 7 api_no_access_2        API No Access 2   <tibble>    <tibble>        nonrepea…
#> 8 survey                 Survey            <tibble>    <tibble>        nonrepea…
#> 9 repeat_survey          Repeat Survey     <tibble>    <tibble>        repeating
#> # ℹ 4 more variables: data_rows <int>, data_cols <int>, data_size <lbstr_by>,
#> #   data_na_pct <formttbl>

New

read_redcap(Sys.getenv("REDCAP_URI"), Sys.getenv("REDCAPTIDIER_CLASSIC_API")) %>%
  suppressWarnings()
#> # A REDCapTidieR Supertibble with 9 instruments
#>   redcap_form_name       redcap_form_label redcap_data redcap_metadata structure
#>   <chr>                  <chr>             <list>      <list>          <chr>    
#> 1 nonrepeated            Nonrepeated       <tibble>    <tibble>        nonrepea…
#> 2 nonrepeated2           Nonrepeated2      <tibble>    <tibble>        nonrepea…
#> 3 repeated               Repeated          <tibble>    <tibble>        repeating
#> 4 data_field_types       Data Field Types  <tibble>    <tibble>        nonrepea…
#> 5 text_input_validation… Text Input Valid… <tibble>    <tibble>        nonrepea…
#> 6 api_no_access          API No Access     <tibble>    <tibble>        nonrepea…
#> 7 api_no_access_2        API No Access 2   <tibble>    <tibble>        nonrepea…
#> 8 survey                 Survey            <tibble>    <tibble>        nonrepea…
#> 9 repeat_survey          Repeat Survey     <tibble>    <tibble>        repeating
#> # ℹ 4 more variables: data_rows <int>, data_cols <int>, data_size <lbstr_by>,
#> #   data_na_pct <formttbl>

Issue Addressed

Closes #157

PR Checklist

Before submitting this PR, please check and verify below that the submission meets the below criteria:

  • New/revised functions have associated tests
  • New/revised functions that update downstream outputs have associated static testing files (.RDS) updated under inst/testdata/create_test_data.R
  • New/revised functions use appropriate naming conventions
  • New/revised functions don't repeat code
  • Code changes are less than 250 lines total
  • Issues linked to the PR using GitHub's list of keywords
  • The appropriate reviewer is assigned to the PR
  • The appropriate developers are assigned to the PR
  • Pre-release package version incremented using usethis::use_version()

Code Review

This section to be used by the reviewer and developers during Code Review after PR submission

Code Review Checklist

  • I checked that new files follow naming conventions and are in the right place
  • I checked that documentation is complete, clear, and without typos
  • I added/edited comments to explain "why" not "how"
  • I checked that all new variable and function names follow naming conventions
  • I checked that new tests have been written for key business logic and/or bugs that this PR fixes
  • I checked that new tests address important edge cases

@ezraporter ezraporter added the bug Something isn't working label Mar 12, 2024
@ezraporter ezraporter requested a review from rsh52 March 12, 2024 18:02
@ezraporter ezraporter self-assigned this Mar 12, 2024
@ezraporter ezraporter requested a review from skadauke March 12, 2024 18:02
@skadauke
Copy link
Collaborator

I think this looks awesome! One tiny thing: please capitalize the final R in REDCapTidieR. :)

Copy link
Collaborator

@rsh52 rsh52 left a comment

Choose a reason for hiding this comment

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

Looks great to me, one minor comment about roxygen docs. I can't think of much else right now to apply this to, but it's good to know we can make use of it.

R/supertibble.R Show resolved Hide resolved
R/supertibble.R Show resolved Hide resolved
@rsh52
Copy link
Collaborator

rsh52 commented Mar 12, 2024

Looks like the failure is a snapshot update which is expected.

The warnings weren't appearing for me but seem to be related to .data for a few instances I used in clean_redcap_long(). I think here:

filter(.data$rep_and_nonrep & !str_ends(.data$field_name, "_form_complete")) %>%
left_join(db_metadata_long %>% select(.data$field_name, .data$form_name),

@ezraporter
Copy link
Collaborator Author

I think this looks awesome! One tiny thing: please capitalize the final R in REDCapTidieR. :)

Good catch!

@ezraporter
Copy link
Collaborator Author

Looks like the failure is a snapshot update which is expected.

The warnings weren't appearing for me but seem to be related to .data for a few instances I used in clean_redcap_long(). I think here:

filter(.data$rep_and_nonrep & !str_ends(.data$field_name, "_form_complete")) %>%
left_join(db_metadata_long %>% select(.data$field_name, .data$form_name),

I think I fixed this in the last commit. I'll keep an eye on the CI. I went ahead and just upgraded everything in our renv.lock so package versions locally should be closer to what's running in CI.

@ezraporter ezraporter requested a review from rsh52 March 12, 2024 21:18
@ezraporter ezraporter merged commit 2025421 into main Mar 12, 2024
6 checks passed
@ezraporter ezraporter deleted the supertibble-style branch March 12, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] rlang change to supertbl label
3 participants