-
Notifications
You must be signed in to change notification settings - Fork 88
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
refactor model specification printing #739
Conversation
note that the only changes needed to pass existing tests is 4 snapshot updates, where `"Model "` is added preceding `"Specification"` in the header.
This is related to #567 |
Oh, sweet! Closes #567. You noted that same edge case, too. :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one! Thanks for the commit order, that made it a lot easier indeed 🙌
Emil's comment #567 (comment) about the print method for the null model is still open. If we want this PR to close #567, we should address this or turn it into its own issue so that it doesn't get lost.
Ah, thanks for pointing that out. It looks like the output of library(parsnip)
null_mod <- null_model()
class(null_mod)
#> [1] "null_model" "model_spec"
methods(class = "null_model")
#> no methods found
methods(class = "nullmodel")
#> [1] predict print tidy
#> see '?methods' for accessing help and source code
# dispatches to new method :(
null_mod
#> null model Model Specification (classification) Created on 2022-06-01 by the reprex package (v2.0.1) I could |
I think |
It does🐨 library(parsnip)
null_model()
#> Null Model Specification (classification) Created on 2022-06-01 by the reprex package (v2.0.1) |
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
While working on some previous PRs, I've noticed that all but one or two of our model spec print methods take the form:
where the only line that changes method-to-method is the
cat
header summarizing the model type.This PR refactors model specification print methods to all dispatch to
print.model_spec
, which now makes use of aprint_model_spec
helper that subsets a tibble giving the descriptions for each class to change that model type description in the header. Hopefully we find this to be more maintainable.This change should be non-breaking for dependencies.
"Model "
is added preceding"Specification"
in the header in 4 print methods.)print.model_spec
won't be dispatched to.This PR still exports the new
print_model_spec
helper, though, in case dependencies want to make use of it. That code would look like:This PR will be hard to review as a whole (lots of deletions across many files), so I tried to commit in such a way to make reviewing commit-by-commit much easier. :)