-
Notifications
You must be signed in to change notification settings - Fork 1k
bug fix: truncate wide columns in print.data.table #7718 #7758
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: master
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -199,12 +199,9 @@ has_format_method = function(x) { | |
| } | ||
|
|
||
| format_col.default = function(x, ...) { | ||
| if (!is.null(dim(x))) | ||
| "<multi-column>" | ||
| else if (is.list(x)) | ||
| vapply_1c(x, format_list_item, ...) | ||
| else | ||
| format(char.trunc(x), ...) # relevant to #37 | ||
| if (!is.null(dim(x))) return("<multi-column>") | ||
| if (is.list(x)) x = vapply_1c(x, format_list_item, ...) | ||
| format(char.trunc(x), ...) | ||
| } | ||
|
|
||
| # #2842 -- different columns can have different tzone, so force usage in output | ||
|
|
@@ -247,12 +244,13 @@ format_list_item.data.frame = function(x, ...) { | |
| # Current implementation may have issues when dealing with strings that have combinations of full-width and half-width characters, | ||
| # if this becomes a problem in the future, we could consider string traversal instead. | ||
| char.trunc = function(x, trunc.char = getOption("datatable.prettyprint.char")) { | ||
| if (is.null(trunc.char)) trunc.char = getOption("width") - 5L | ||
|
Member
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. just curious, why 5L? and this looks like a magic number, could we use a constant instead?
Contributor
Author
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. my major reason of using 5L was because toby sir mentioned this
Member
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. would 5L still make sense if we had a large table and the row label prefix consists of many more characters? I would argue against a magic number or even reading the row label prefix if we can here
Contributor
Author
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. you are right the 5L buffer fails once row labels grow bigger on a narrow consoles.
Member
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. can you please provide a couple examples of when 5L works and does not work?
Contributor
Author
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. its working fine for your example ,i was accounting about the final width because the 5L bugffer doent not account all layout components (row labels,spacing,'...') if you count the truncated data only then its behaving correctly , however if teh fully rendered line is considered then its excedding the 25 limit.
Member
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. thanks. options(width) is documented on ?options as a limit on the whole line (the maximum number of columns on a line used in printing vectors, matrices and arrays, and when filling by cat), so in this example the output should be 25 characters,
Contributor
Author
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. can you guide me towards what could be the next step.
Member
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’m not sure, I have not thought it through. |
||
| trunc.char = max(0L, suppressWarnings(as.integer(trunc.char[1L])), na.rm=TRUE) | ||
| if (!is.character(x) || trunc.char <= 0L) return(x) | ||
| nchar_width = nchar(x, 'width') # Check whether string is full-width or half-width, #5096 | ||
| nchar_chars = nchar(x, 'char') | ||
| nchar_width = nchar(x, 'width', allowNA=TRUE) # Check whether string is full-width or half-width, #5096 | ||
| nchar_chars = nchar(x, 'char', allowNA=TRUE) | ||
| is_full_width = nchar_width > nchar_chars | ||
| idx = !is.na(x) & pmin(nchar_width, nchar_chars) > trunc.char | ||
| idx = !is.na(x) & !is.na(nchar_width) & pmin(nchar_width, nchar_chars) > trunc.char | ||
|
Member
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. could we use na.rm in pmin instead of a guard?
Contributor
Author
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 tried using |
||
| x[idx] = paste0(strtrim(x[idx], trunc.char * fifelse(is_full_width[idx], 2L, 1L)), "...") | ||
| x | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -21620,3 +21620,20 @@ test(2373.1, empty_dt[, 1, by = numeric()], data.table(numeric=numeric(), V1=num | |||||||
| test(2373.2, empty_dt[, 1L, by = numeric()], data.table(numeric=numeric(), V1=integer())) | ||||||||
| test(2373.3, empty_dt[, .(x=1), by = .(b=numeric())], data.table(b=numeric(), x=numeric())) | ||||||||
| test(2373.4, data.table()[, x := 1, by=numeric()], data.table(x=numeric())) | ||||||||
|
|
||||||||
| # issue 7718 | ||||||||
| # print.data.table truncates long character columns based on width | ||||||||
| options(width=10, datatable.prettyprint.char=NULL) | ||||||||
| test(2374.1, capture.output(print(data.table(x="12345678901234567890"))), output="12345...") | ||||||||
|
Comment on lines
+21626
to
+21627
Member
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. please use test(options=) instead of options(…), here and below
Suggested change
|
||||||||
| # list columns are truncated to fit console width | ||||||||
| options(width=15, datatable.prettyprint.char=NULL) | ||||||||
| test(2374.2, capture.output(print(data.table(L=list(1:20)))) , output="1,2,3,4,5,...") | ||||||||
| # long strings are truncated while short strings are preserved | ||||||||
| options(width=20, datatable.prettyprint.char=NULL) | ||||||||
| test(2374.3, capture.output(print(data.table(x=c("short", "abcdefghijklmnopqrstuvwxyz")))), output="abcdefghijklmno...") | ||||||||
| # large widths print full character values without truncation | ||||||||
| options(width=200, datatable.prettyprint.char=NULL) | ||||||||
| test(2374.4, capture.output(print(data.table(x="abcdefghijklmnopqrstuvwxyz"))), output="abcdefghijklmnopqrstuvwxyz") | ||||||||
| # mixed numeric and character columns preserve formatting while truncating long text | ||||||||
| options(width=20, datatable.prettyprint.char=NULL) | ||||||||
| test(2374.5, capture.output(print(data.table(id=1L, score=99.12345, txt="abcdefghijklmnopqrstuvwxyz"))), output="abcdefghijklmno...") | ||||||||
Uh oh!
There was an error while loading. Please reload this page.