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

Add strict option for functions #34

Closed
ddsjoberg opened this issue Sep 1, 2020 · 2 comments · Fixed by #37
Closed

Add strict option for functions #34

ddsjoberg opened this issue Sep 1, 2020 · 2 comments · Fixed by #37

Comments

@ddsjoberg
Copy link
Collaborator

As a developer, it would be helpful to have the option for some broom.helper functions to fail when they cannot execute the requested action.

I am integrating broom.helpers into gtsummary now, and these two scenarios have come up so far:

  1. When I run broom.helpers::tidy_identify_variables() if the variables cannot be identified, I would like to be able to have the function error. As it is currently written, I would need to inspect the returned object to check if the variables were indeed identified.

  2. When I run broom.helpers::tidy_add_header_rows(show_single_row=) for a variable that cannot be put on a single row.

Perhaps the arg could be something like tidy_plus_plus(strict=)? It would be similar to how purrr had pluck() and chuck()?

@larmarange
Copy link
Owner

I see. It could a good option and strict is I think the good argument name.
However, it should be generalized to each tidy_* functions.

  • tidy_identify_variables(): check if variable names has been found. If it is not the case, var_type will be "unknown".
  • tidy_add_contrasts(): check, if there are categorical variables only, that a contrasts value has been documented. Do we have a case where it will fail?
  • tidy_add_reference_rows(): all categorical variables with a treatment, SAS or sum contrasts should have a reference row. Not sure if there is a case where it will fail.

and so on....

A difficulty is to identify cases where it will fail to develop the appropriate tests.

Not sure to have enough time this week but if you want to explore and to propose a PR, you are welcome

@ddsjoberg
Copy link
Collaborator Author

I like adding the strict argument for tidy_identify_variables() and tidy_add_header_rows(show_single_row=) because the user is specifically asking for something that can result in an error.

For tidy_add_contrasts() I would only make it an error if there was a categorical variable that should have a contrast and it cannot be addressed.

For tidy_add_reference_rows() I would only make it an error if there was a categorical variable that should have a reference row that we cannot add.

I'll work on a PR.

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 a pull request may close this issue.

2 participants