Add print methods for control objects with formatted output using cli#1108
Add print methods for control objects with formatted output using cli#1108tjburch wants to merge 4 commits intotidymodels:mainfrom
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
This is all nice already! I would suggest, if you have time to spare, to take a look at the cli classes we can use. This should allow you to reduce some of the code you have in |
|
Additional if we wanted to only show what is non-default we could add a Add the following line inside the loop if (identical(value, default[[field]])) {
next
}and lastly pass in the corresponding objects in the uses. So print.control_grid <- function(x, ...) {
cli::cli_text("{.emph Grid/resamples control object}")
--- print_control_settings(x)
+++ print_control_settings(x, default = control_grid())
invisible(x)
}I'm looping @topepo in on this decision as i don't have very strong feelings here. |
|
Thanks - implemented. The function print is still kind of ugly, but the rest now use the cli classes better. |
|
I thought that we could do something fancy with |
tests/testthat/test-checks.R
Outdated
| expect_s3_class(control_bayes(), "control_bayes") | ||
| }) | ||
|
|
||
| test_that("control object print methods", { |
There was a problem hiding this comment.
I think that this should live in test-control.R
| cli::cli_bullets(c(" " = "{.arg {field}}: <function>")) | ||
| } else if (inherits(value, "tune_backend_options")) { | ||
| cli::cli_bullets(c(" " = "{.arg {field}}: <backend_options>")) | ||
| } else { |
There was a problem hiding this comment.
Can we add a branch for NULL values before the last that is:
else if (is.null(value)) {
cli::cli_bullets(c(" " = "{.arg {field}}: NULL"))
}We can then get the output without the empty spaces:
Grid/resamples control object
`verbose`: FALSE
`allow_par`: TRUE
`extract`: NULL
`save_pred`: FALSE
`pkgs`: NULL
`save_workflow`: FALSE
`event_level`: "first"
`parallel_over`: NULL
`backend_options`: NULL
`workflow_size`: 100
There was a problem hiding this comment.
Done in latest commit
- Add NULL value handling to print_control_settings function This prevents empty output for NULL values, showing them explicitly as "NULL" - Move control object print method tests from test-checks.R to test-control.R for better organization - Remove corresponding snapshots from checks.md (will be regenerated in control.md) These changes address topepo's review feedback on PR tidymodels#1108.
|
Cool, just implemented NULL handling in the latest commit and put tests in the right place. Let me know if there's anything else. Ignore the claude reference above - got some free credits dumped into my account that I figured I'd see what it could do and didn't realize it would go off and put its changes on git. |
Closes #1051 by adding a cli-based printout of control objects. Went pretty simple, just a cli bulleted list of all parameters. Doing all is up for debate - I figure it's a bit nicer in case users don't know defaults off the cuff, but can see the argument for only non-default values, the prints do get a little verbose.