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.

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
Show file tree
Hide file tree
Changes from all 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
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, @sritchie73 for fixing, and @HughParsonage for feedback

#### NOTES


Expand Down
9 changes: 7 additions & 2 deletions R/data.table.R
Original file line number Diff line number Diff line change
Expand Up @@ -1887,14 +1887,19 @@ chmatch2 <- function(x, table, nomatch=NA_integer_) {
# x
#}

as.matrix.data.table <- function(x, rownames, ...) {
as.matrix.data.table <- function(x, rownames, rownames.literal=FALSE, ...) {
Copy link
Member

Choose a reason for hiding this comment

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

In such unwanted edge case we don't have to be consistent to data.frame by default, it is important to give option for consistency to data.frame, which new arg already provides, but we could tune its default.
Maybe new arg could have smart default like nrow(x)==1L? probably more correct would be nrow(x)==1L && !missing(rownames) && is.character(rownames) but it is a little bit messy, maybe nrow(x)==1L is sufficient in this case.

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 (nrow(x) == 1 && isTRUE(rownames.literal)) {
Copy link
Member

@jangorecki jangorecki Jun 17, 2018

Choose a reason for hiding this comment

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

don't we need length(rownames) == 1L here?

# 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
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", rownames.literal=TRUE), 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
7 changes: 6 additions & 1 deletion man/as.matrix.Rd
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Converts a \code{data.table} into a \code{matrix}, optionally using one
of the columns in the \code{data.table} as the \code{matrix} \code{rownames}.
}
\usage{
\method{as.matrix}{data.table}(x, rownames, ...)}
\method{as.matrix}{data.table}(x, rownames, rownames.literal, ...)}

\arguments{
\item{x}{a \code{data.table}}
Expand All @@ -17,6 +17,11 @@ the \code{rownames} in the returned \code{matrix}. If \code{TRUE} the
single column, otherwise the first column in the \code{data.table} will
be used. Alternative a vector of length \code{nrow(x)} to assign as the
row names of the returned \code{matrix}.}
\item{rownames.literal}{optional, when x is a \code{data.table} that has
Copy link
Member

Choose a reason for hiding this comment

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

before word "optional" I would put "logical" to clearly state argument type in front, so {logical, optional, when ....

x alone should be wrapped in \code{x}

... when x is ...

to

... when \code{x} is ...

only a single row, set \code{rownames.literal} to \code{TRUE} if you want
to use the vector supplied to the \code{rownames} argument as the row
names of the returned \code{matrix} instead of looking for \code{rownames}
as a column of \code{x}.}
\item{\dots}{additional arguments to be passed to or from methods.}
}

Expand Down