Add measures parameter to data_tabulate()#689
Conversation
| } | ||
|
|
||
| out$`Raw %` <- 100 * out$N / sum(out$N) | ||
| if ("raw" %in% measures){ |
There was a problem hiding this comment.
Please use space around infix-operators :-)
There was a problem hiding this comment.
This is fixed, not sure why it doesn't show as outdated.
| #' @param measures Optional character vector, indicating the types of | ||
| #' percents to be included. Only applies to frequencies, i.e. when `by` is | ||
| #' `NULL`. Can be `"raw"` (includes `NA` values), `"valid"` (excludes `NA` values) | ||
| #' or `"cumulative"` (excludes `NA` vlues). |
There was a problem hiding this comment.
On top of the other comments to address above, please add a bullet point in NEWS.md and add an example for the new argument in the docs of data_tabulate().
(@strengejacke I hid all the copilot comments because they were just duplicates of your comments and adding noise to this PR)
|
I was wondering if it should be documented that |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #689 +/- ##
=======================================
Coverage 92.09% 92.10%
=======================================
Files 76 76
Lines 7727 7736 +9
=======================================
+ Hits 7116 7125 +9
Misses 611 611 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
etiennebacher
left a comment
etiennebacher
left a comment
There was a problem hiding this comment.
I was wondering if it should be documented that measures = c() will return the frequencies only.
Yes I think so.
Also, that made me wonder if "N" should also be optional.
I don't know about that but if it's not included then I think this measures parameter should be renamed shares. @strengejacke WDYT?
The workflow for code styling fails. We use Air to automatically format code so you can install it and run it on the project to fix this failure.
| #' # exclude the cumulative percent column | ||
| #' data_tabulate(efc$c172code, remove_na = TRUE) | ||
| #' |
There was a problem hiding this comment.
The new example should show how to use measures instead of remove_na.
| #' `NULL`. Can be `"raw"` (includes `NA` values), `"valid"` (excludes `NA` values) | ||
| #' or `"cumulative"` (excludes `NA` values). |
There was a problem hiding this comment.
| #' `NULL`. Can be `"raw"` (includes `NA` values), `"valid"` (excludes `NA` values) | |
| #' or `"cumulative"` (excludes `NA` values). | |
| #' `NULL`. Can be several choices among `"raw"` (includes `NA` values), | |
| #' `"valid"` (excludes `NA` values) and `"cumulative"` (excludes `NA` values). |
| ) | ||
| }) | ||
|
|
||
| test_that("data_tabulate data.frame with measures", { |
There was a problem hiding this comment.
Please add tests for measures = c(), measures = NULL and measures = "foo".
| * `data_tabulate()` gain a `measures` argument to allow selection of columns to | ||
| display ("raw", "valid", and "cumulative") (#689). |
There was a problem hiding this comment.
You can add your github username if you want: (#689, <username>)
etiennebacher
changed the title
measures parameter to data_tabulate()
There were a few ways to approach this. The cumulative values depends on having the valid values, and it could make sense to just set any not included measures to NULL at the end, the way I did for valid.
I added a test for
measures = "raw"... I could add for other variations also.Fixes #685