-
Couldn't load subscription status.
- Fork 71
obj_print*() gain max argument
#1482
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
6ac33b9
35186b7
dc20934
611f261
4753657
9b806f9
f0877d4
7516e5f
9d0c1fe
ce01ffb
10d5084
667e321
04483a2
0dd0d20
3387df4
c895ff3
6ece3fb
b755345
8ad1bf5
e7fc6eb
7db1d0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,18 +9,26 @@ | |
| #' @param x A vector | ||
| #' @param ... Additional arguments passed on to methods. See [print()] and | ||
| #' [str()] for commonly used options | ||
| #' @param max The maximum number of items to print, defaults to | ||
| #' `getOption("print.max")`. | ||
| #' @keywords internal | ||
| #' @export | ||
| obj_print <- function(x, ...) { | ||
| obj_print_header(x, ...) | ||
| obj_print_data(x, ...) | ||
| obj_print_footer(x, ...) | ||
| obj_print <- function(x, ..., max = NULL) { | ||
| max <- local_max_print(max) | ||
| obj_print_header_dispatch(x, ..., max = max) | ||
| obj_print_data_dispatch(x, ..., max = max) | ||
| obj_print_footer_dispatch(x, ..., max = max) | ||
| invisible(x) | ||
| } | ||
|
|
||
| #' @export | ||
| #' @rdname obj_print | ||
| obj_print_header <- function(x, ...) { | ||
| obj_print_header <- function(x, ..., max = NULL) { | ||
| max <- local_max_print(max) | ||
| return(obj_print_header_dispatch(x, ..., max = max)) | ||
| UseMethod("obj_print_header") | ||
| } | ||
| obj_print_header_dispatch <- function(x, ..., max) { | ||
| UseMethod("obj_print_header") | ||
| } | ||
|
|
||
|
|
@@ -32,32 +40,76 @@ obj_print_header.default <- function(x, ...) { | |
|
|
||
| #' @export | ||
| #' @rdname obj_print | ||
| obj_print_data <- function(x, ...) { | ||
| obj_print_data <- function(x, ..., max) { | ||
krlmlr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| max <- local_max_print(max) | ||
| return(obj_print_data_dispatch(x, ..., max = max)) | ||
| UseMethod("obj_print_data") | ||
| } | ||
| obj_print_data_dispatch <- function(x, ..., max) { | ||
| UseMethod("obj_print_data") | ||
| } | ||
|
|
||
| #' @export | ||
| obj_print_data.default <- function(x, ...) { | ||
| if (length(x) == 0) | ||
| obj_print_data.default <- function(x, ..., max) { | ||
| if (!vec_is(x)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really see many people using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm but I feel like that is mostly for testing, and we don't export it. Any thoughts on this @lionel- ? |
||
| print(x, quote = FALSE) | ||
| return(invisible(x)) | ||
| } | ||
|
|
||
| if (vec_size(x) > max) { | ||
| x_max <- vec_slice(x, seq_len(max)) | ||
| } else { | ||
| x_max <- x | ||
| } | ||
|
|
||
| if (vec_size(x_max) == 0) { | ||
| return(invisible(x)) | ||
| } | ||
|
|
||
| out <- stats::setNames(format(x), names(x)) | ||
| out <- stats::setNames(format(x_max), names(x_max)) | ||
| print(out, quote = FALSE) | ||
|
|
||
| invisible(x) | ||
| } | ||
|
|
||
| #' @export | ||
| #' @rdname obj_print | ||
| obj_print_footer <- function(x, ...) { | ||
| obj_print_footer <- function(x, ..., max = NULL) { | ||
| max <- local_max_print(max) | ||
| return(obj_print_footer_dispatch(x, ..., max = max)) | ||
| UseMethod("obj_print_footer") | ||
| } | ||
| obj_print_footer_dispatch <- function(x, ..., max) { | ||
| UseMethod("obj_print_footer") | ||
| } | ||
|
|
||
| #' @export | ||
| obj_print_footer.default <- function(x, ...) { | ||
| obj_print_footer.default <- function(x, ..., max = NULL) { | ||
krlmlr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (!vec_is(x)) { | ||
| return(invisible(x)) | ||
| } | ||
|
|
||
| delta <- vec_size(x) - max | ||
| if (delta > 0) { | ||
| cat_line("... and ", big_mark(delta), " more") | ||
krlmlr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
krlmlr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| invisible(x) | ||
| } | ||
|
|
||
| local_max_print <- function(max, frame = parent.frame()) { | ||
| max_print <- getOption("max.print") | ||
DavisVaughan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (is.null(max)) { | ||
| return(max_print) | ||
| } | ||
|
|
||
| stopifnot(is_integerish(max, 1L, finite = TRUE), max >= 0) | ||
| if (max > max_print) { | ||
| # Avoid truncation in case we're forwarding to print() | ||
| local_options(max.print = max, .frame = frame) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| } | ||
| max | ||
| } | ||
|
|
||
|
|
||
| # str --------------------------------------------------------------------- | ||
|
|
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
I feel like this whole local function +
_dispatch()redirection is a bit complicated. Can you help me understand why we need it? In clock I was able to getprint()to work correctly after I already slicedxby passingmaxall the way through https://github.com/r-lib/clock/blob/0a07f630bc5859e33b34b85380fafe028cfed5eb/R/calendar.R#L21, can we do that here instead?Then I'd imagine we just pass
maxthrough to each of the three helpers, they'd each validatemaxusing some helper like this one https://github.com/r-lib/clock/blob/f3b79db9226fd4af09b1b7175bdfbb4d225386fb/R/utils.R#L263, and then use it as requiredIf it has something to do with making it easier for subclasses who implement
obj_print_data()methods, I'm not sure we should worry about that too much, since if you implement aobj_print_data()method then you'd already have to handle the slicing yourself anyways, so you may as well handle the collection of themaxargument tooThere 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.
With
local_max_print()and_dispatchwe:maxarg with a consistent error messagemaxis set to a value larger thangetOption("max.print")getOption("max.print")in every helper (we need it at least in the data and in the footer)I rather like this pattern. Slicing really is just one line of code, handling
maxis like 15 in an extra helper function.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.
(*) if we had a
vec_head()We could also handle the slicing ourselves, but then we'd need to pass a
sizeargument. Also, some vctrs classes might be able to print without slicing explicitly.