Skip to content
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

Closes #2930 -- bugfix to as.matrix.data.table() #2938

Closed
wants to merge 26 commits into from
Closed

Closes #2930 -- bugfix to as.matrix.data.table() #2938

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0389de1
rownames argument to as.matrix and as.data.frame
sritchie73 Mar 22, 2018
b1590ae
Added documentation
sritchie73 Mar 23, 2018
1323be0
some unit tests
sritchie73 Mar 23, 2018
ddaeb6a
added news
sritchie73 Mar 23, 2018
0ab7e4f
Resolve CRAN notes about global variable bindings
sritchie73 Mar 23, 2018
8477788
Reverted as.data.frame.data.table
sritchie73 Mar 23, 2018
ac52d9a
Unit tests for errors and warnings
sritchie73 Mar 24, 2018
b9eab65
Fixed error message in tests and test increment numbers
sritchie73 Mar 24, 2018
c0cca0d
Merging changes from upstream
sritchie73 Mar 24, 2018
11da144
Removed with=FALSE
sritchie73 Mar 25, 2018
5b4bca7
Enhanced clarity of an error check
sritchie73 Mar 25, 2018
c5ae94b
replaced isTRUE
sritchie73 Mar 25, 2018
f07b813
Vector of rownames may be used in as.matrix
sritchie73 Mar 25, 2018
3d4681c
Fixed number in tests to reflect PR
sritchie73 Mar 25, 2018
d9a4a54
mainly cosmetic changes
MichaelChirico Mar 27, 2018
de48d84
typo (rownames is integer not string here)
MichaelChirico Mar 27, 2018
2810538
Merge branch 'master' into as_matrix_rownames
mattdowle Mar 30, 2018
8348172
merged master
sritchie73 Apr 1, 2018
12cace8
merge upstream PR changes
sritchie73 Apr 1, 2018
895554e
NULL, FALSE, and NA now passthrough instead of warning
sritchie73 Apr 1, 2018
a887594
Fixed incorrect warning message
sritchie73 Apr 1, 2018
cde74a2
Removed extraneous newline before tests
sritchie73 Apr 3, 2018
6afb012
Merge branch 'master' into as_matrix_rownames
sritchie73 Jun 17, 2018
5be4459
Fix for Issue #2930
sritchie73 Jun 17, 2018
3666b9c
Added argument rownames.literal to as.matrix()
sritchie73 Jun 17, 2018
9add536
Fixed docs
sritchie73 Jun 17, 2018
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 NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#### BUG FIXES

1. Passing a column to use as the rownames to `as.matrix.data.table()` now works when the input `data.table` has a single row, [#2930](https://github.com/Rdatatable/data.table/issues/2930). Thanks to @malcook for reporting and @sritchie73 for fixing.

#### NOTES


Expand Down
7 changes: 6 additions & 1 deletion R/data.table.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1891,10 +1891,15 @@ as.matrix.data.table <- function(x, rownames, ...) {
rn <- NULL
rnc <- NULL
if (!missing(rownames)) { # Convert rownames to a column index if possible
if (length(rownames) == nrow(x)) {
if (length(rownames) == nrow(x) && nrow(x) > 1) {
# rownames argument is a vector of row names, no column in x to drop.
rn <- rownames
rnc <- NULL
} else if (length(rownames) == nrow(x) && nrow(x) == 1 && is.character(rownames) && !(rownames %in% colnames(x))) {
Copy link
Member

Choose a reason for hiding this comment

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

I think an extra argument for the length-1 case is going to be necessary. Something like rownames.literal = TRUE.

as.matrix(data.table(x = 1, z = 2), "z")
#>   x z
#> z 1 2
as.matrix(data.table(x = 1, z = 2), "z", rownames.literal = FALSE)
#>   x z
#> 2 1 2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. I'm not sure having an extra argument is really necessary, since it will only apply to data.tables with 1 row, unless we want to allow vector recycling for the rownames when the data.table has > 1 rows.

Copy link
Member

Choose a reason for hiding this comment

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

But this PR arose because of confusion about 1-row data.tables. And there may be other cases where the overloaded argument of rownames might cause problems.

# When x has a single row, the user may still want to supply vector of rownames,
# but we need to distinguish from the case when the rownames is a column of x.
rn <- rownames
rnc <- NULL
} else if (!is.null(rownames) && length(rownames) != 1L) { # vector(0) will throw an error, but NULL will pass through
stop(sprintf("rownames must be a single column in x or a vector of row names of length nrow(x)=%d", nrow(x)))
} else if (!(is.null(rownames) || is.logical(rownames) || is.character(rownames) || is.numeric(rownames))) {
Expand Down
6 changes: 6 additions & 0 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11702,6 +11702,12 @@ test(1899.12, as.matrix(DT, FALSE), mat2)
setkey(DT, id, X)
test(1899.13, as.matrix(DT, TRUE), mat, warning="rownames is TRUE but multiple keys")

# Check handling of cases where the data.table only has 1 row, raised by Issue #2930:
mat4 <- matrix(c("a", 1, 5), nrow=1, dimnames=list(c("x"), c("id", "X", "Y")))
test(1899.14, as.matrix(DT[1,], 1), mat[1,,drop=FALSE])
test(1899.15, as.matrix(DT[1,], "id"), mat[1,,drop=FALSE])
test(1899.16, as.matrix(DT[1,], "x"), mat4) # "x" not a column in DT, so use "x" as the rownames

# index argument for fread, #2633
DT_str = c('a,b\n3,1\n2,2\n1,1\n2,1\n3,2')
test(1900.1, attributes(attr(fread(DT_str, index = 'a'), 'index')),
Expand Down