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

try parameters if tiyd_fun fail? #104

Closed
larmarange opened this issue Feb 19, 2021 · 6 comments
Closed

try parameters if tiyd_fun fail? #104

larmarange opened this issue Feb 19, 2021 · 6 comments

Comments

@larmarange
Copy link
Owner

  • add a tidy_parameters() custom tieders using parameters instead of broom
  • in tidy_and_attach(), if tidy_fun fails, try tidy_parameters and show a message
@larmarange
Copy link
Owner Author

@ddsjoberg I have added a tidy_parameters() custom tidier based on parameters::model_parameters() in #111 because some models are not supported by tidy().

Regarding tidy_and_attach(), for simplifying the use for most users, I was thinking that for most users it would be good to tryCatch tidy() and, if not successful, to use tidy_parameters().

I see two possibilities:

  • create a tidy_with_broom_or_paramaters() tidier implementing this tryCatch and defining this tidier as default value for tidy_fun
  • or updating tidy_and_attach() to try tidy_fun first and then try tidy_paramters(). However, I have the feeling that it is less explicit and when someone pass a custom tieder we should not go back on tidy_parameters() in case of failure.

What do you think?

@ddsjoberg
Copy link
Collaborator

I think that is awesome! I agree with you that if a user passes a specific tidier, then that one and only that tidier should be used.

Maybe the default should be updated to tidy_fun = NULL with

if (is.null(tidy_fun)) result <- tidy_with_broom_or_paramaters(x)
else result <- tidy_fun(x)

I also think some messaging would be helpful. For example, if broom::tidy() fails print a message that tidy failed and trying parameters?

In tbl_regression() I populate tidy_fun= broom::tidy, so I may need to update as well to take advantage of the new functionality

larmarange added a commit that referenced this issue Mar 10, 2021
@larmarange
Copy link
Owner Author

@ddsjoberg I have drafted tidy_with_broom_or_parameters()

For now, in all cases, it returns a message explaining which tieder was used. Should I add a quiet option or a message explaining to provide an explicit tidier to turn the message off?

For tidy_plus_plus(), instead of using NULL as default, I simply changed the default to tidy_with_broom_or_parameters(). I have the feeling that it would be more explicit and visible.

What do you think?

@ddsjoberg
Copy link
Collaborator

For now, in all cases, it returns a message explaining which tieder was used. Should I add a quiet option or a message explaining to provide an explicit tidier to turn the message off?

hmm, good question. I would suggest one of two ways forward. 1. No messaging when broom::tidy() works. But when it fails and parameters succeeds add this message, "The broom::tidy(x)errored, andparameters::model_parameters()was used instead. Addtidy_fun = parameters::model_parameters to quiet this message." (I think the code passed in tidy_fun would have the extra step/argument to make it tidy format). Or, 2. Add a quiet argument. I would probably set this to always be quiet though to avoid the messaging when broom::tidy is used, which is the vast majority of the time. I do like letting users know what is being used to tidy their model results, and i think it's needed.

For tidy_plus_plus(), instead of using NULL as default, I simply changed the default to tidy_with_broom_or_parameters(). I have the feeling that it would be more explicit and visible.

That sounds good to me. Will you also export that function, so I can use it in gtsummary?

@larmarange
Copy link
Owner Author

Yes the function will be exported.

I will update the messages.

@larmarange
Copy link
Owner Author

now fixed

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

No branches or pull requests

2 participants