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

Adding 'card' print method #102

Merged
merged 8 commits into from
Jan 5, 2024
Merged

Adding 'card' print method #102

merged 8 commits into from
Jan 5, 2024

Conversation

ddsjoberg
Copy link
Collaborator

@ddsjoberg ddsjoberg commented Jan 4, 2024

What changes are proposed in this pull request?
Adding print.card() method.

@bzkrouse what do you think of the print method? Once we finalize, I'll finish the PR which will lead to some changes in snapshot testing I think.

image

What the print method does:

  • converts object to data frame so the list columns show the values (unlike a tibble print)
  • prints between 10 and 20 rows
  • messaging for the number of rows/columns that are not shown
  • Removes warning and error columns if nothing to report
  • if there are more than 6 columns, statistic_fmt_fn/contex columns removed
  • A max of 9 characters are displayed for columns 'group##_level', 'variable_level', 'stat_label', and 'context'
  • If a statistic is a double, then it is rounded to 3 decimal places
  • If a proper function is found in the statistic_fmt_fn column, it's printed as "<fn>" to save space

Reference GitHub issue associated with pull request. e.g., 'closes #1'
closes #93


Reviewer Checklist (if item does not apply, mark is as complete)

  • Ensure all package dependencies are installed: devtools::install_dev_deps()
  • PR branch has pulled the most recent updates from master branch: usethis::pr_merge_main()
  • If a bug was fixed, a unit test was added.
  • Run pkgdown::build_site(). Check the R console for errors, and review the rendered website.
  • Code coverage is suitable for any new functions/features: devtools::test_coverage()
  • usethis::use_spell_check() runs with no spelling errors in documentation

When the branch is ready to be merged:

  • Update NEWS.md with the changes from this pull request under the heading "# cards (development version)". If there is an issue associated with the pull request, reference it in parentheses at the end update (see NEWS.md for examples).
  • Increment the version number using usethis::use_version(which = "dev")
  • Run usethis::use_spell_check() again
  • Approve Pull Request
  • Merge the PR. Please use "Squash and merge".

@ddsjoberg ddsjoberg marked this pull request as ready for review January 5, 2024 00:42
@ddsjoberg ddsjoberg requested a review from bzkrouse January 5, 2024 00:42
@ddsjoberg
Copy link
Collaborator Author

I think this one is ready for review now, @bzkrouse ! Thanks!

Copy link
Collaborator

@bzkrouse bzkrouse left a comment

Choose a reason for hiding this comment

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

This is really nice!!

README.md Outdated
#> 4 ARM Placebo AGEGR1 >80 p % 0.349
#> 5 ARM Placebo AGEGR1 65-80 n n 42
#> 6 ARM Placebo AGEGR1 65-80 p % 0.488
#> 7 ARM Placebo AGEGR1 N N 86
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the readme needs re-rendering after the last update :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ahhh good catch! thank you!

@ddsjoberg ddsjoberg merged commit 0ebb96b into main Jan 5, 2024
9 checks passed
@ddsjoberg ddsjoberg deleted the view_ard branch January 5, 2024 20:57
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.

Write a print method for the cards data frames
2 participants