R4EPI / epitabulate

Tables for epidemiological analysis
https://R4EPI.github.io/epitabulate
GNU General Public License v3.0
8 stars 0 forks source link

Update tidyselect to version 1.0.0 #9

Closed zkamvar closed 4 years ago

zkamvar commented 4 years ago

I've done the following

zkamvar commented 4 years ago

@aspina7, is this going to be a major booboo if someone is not notified if they use an incorrect variable name?

Example here tries to tabulate the cyl and horses columns from mtcars, but it does not contain a horses column. No warning is thrown that the horses were not used.

library("epibuffet")
tab_linelist(mtcars, cyl, horses)
#> Warning: converting `cyl` to a factor
#> # A tibble: 3 x 4
#>   variable value     n proportion
#>   <chr>    <chr> <dbl>      <dbl>
#> 1 cyl      4        11       34.4
#> 2 cyl      6         7       21.9
#> 3 cyl      8        14       43.8

Created on 2020-02-10 by the reprex package (v0.3.0)

aspina7 commented 4 years ago

hmmmm guess a warning would be nice .... ? Otherwise ppl gonna be wondering why there isnt output for x number of vars they spelt wrong? Guess an easy fixeroo would be if the num unique(variable) in output is not same length as arguments put in? Then could give a nudge to check spelling?

zkamvar commented 4 years ago

hmmmm guess a warning would be nice .... ? Otherwise ppl gonna be wondering why there isnt output for x number of vars they spelt wrong? Guess an easy fixeroo would be if the num unique(variable) in output is not same length as arguments put in? Then could give a nudge to check spelling? it's not that easy of a fix.

In the templates, we have the users supply a character vector of symptoms:

symptoms <- c("mpg", "horses")
tab_linelist(mtcars, symptoms)

The problem is that they could also use a combination of different helpers like starts_with(), etc. Moreover, the functions that actually parse out these arguments take either a strict or non-strict approach with no in-between, so you either get an error or a subset of what worked. At the moment, the errors are inconsistent, so it's difficult for me to parse these things.

aspina7 commented 4 years ago

then screwed if we do, screwed if we dont .... So whichever is easier code-wise!

zkamvar commented 4 years ago

Note that we should update this in the sitrep package templates so that they use the any_of(SYMPTOMS) instead of SYMPTOMS because it's ambiguous as to whether SYMPTOMS refers to a column name (error) or a vector in the environment that gives the column names. It returns a warning now in the new version of tidyselect and will most likely become an error next year.

aspina7 commented 4 years ago

will tag on to end of one of the pat lists