Skip to content

Fix: Ensure column widths are data-driven when col.names = "none" in print.data.table #6916

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.

Sign up for GitHub

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

Closed
venom1204 wants to merge 6 commits into from
Closed

Fix: Ensure column widths are data-driven when col.names = "none" in print.data.table #6916

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b15f235
coloumn width adjusted
venom1204 Apr 13, 2025
73263c3
correctedchanges
venom1204 Apr 22, 2025
4db3948
updated condition
venom1204 Apr 17, 2025
b70987f
lintr update
venom1204 Apr 17, 2025
fbc0029
operator correction
venom1204 Apr 22, 2025
29f14fd
resolve conflicts
venom1204 Jul 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions R/print.data.table.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ print.data.table = function(x, topn=getOption("datatable.print.topn"),
print_default(toprint)
return(invisible(x))
}
if (col.names == "none")
Copy link
Member

@MichaelChirico MichaelChirico Apr 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you try making this change on lines 94-96 above instead?

Copy link
Contributor Author

@venom1204 venom1204 Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i forgot to include ,i will try it now

Copy link
Contributor Author

@venom1204 venom1204 Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelChirico I tried implementing it
However, when I tried to integrate this with the existing column name clearing logic as you suggested
Tests 2125.06 and 2125.07 fail, which involve truncation messages when col.names="none".

The issue appears to be related to how the truncation message logic works in trunc_cols_message(). When column names are cleared early in the combined condition, the truncation logic can no longer access the original column names, resulting in empty brackets in messages:

  • Expected: 1 variable not shown: [d]
  • Observed: 1 variable not shown: []

With the standalone implementation, the column names are likely cleared after the truncation message logic has already determined which columns to display, maintaining the correct behavior.

Copy link
Member

@joshhwuu joshhwuu Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could maybe store the removed column names temporarily and then make the change?

TBH if I used col.names == "none" I would not expect a x variable(s) not shown: [cols] printout, as I didn't want to see column names in the first place

colnames(toprint) = rep.int("", ncol(toprint))
if (nrow(toprint)>20L && col.names == "auto")
# repeat colnames at the bottom if over 20 rows so you don't have to scroll up to see them
# option to shut this off per request of Oleg Bondar on SO, #1482
Expand Down
13 changes: 13 additions & 0 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21141,3 +21141,16 @@ dt = data.table(id = 1:25)
test(2314.1, any(grepl("<int>", tail(capture.output(print(dt, class = TRUE)), 2))), TRUE)
# Test that class=TRUE with col.names="top" doesn't show classes at bottom
test(2314.2, !any(grepl("<int>", tail(capture.output(print(dt, class = TRUE, col.names = "top")), 2))), TRUE)

test(2315.1, {
dt <- data.table(short=1:3, verylongcolumnname=4:6)
print(dt, col.names="none")
}, output="1: 1 4\n2: 2 5\n3: 3 6")
test(2315.2, {
dt <- data.table(x=123456, y="wide_string")
print(dt, col.names="none")
}, output="1: 123456 wide_string")
test(2315.3, {
dt <- data.table(a=NA_integer_, b=NaN)
print(dt, col.names="none")
}, output="1: NA NaN\n")
Copy link
Contributor

@Mukulyadav2004 Mukulyadav2004 May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of small suggestions to align with the style used in inst/tests/tests.Rraw:

  • Initialize data.table outside the test() call.
  • No need for {}, since each test() contains only a single expression.
  • Add a comment above the test block like:
    # Tests for print() output with fixed index header (PR #6806)

Loading