Skip to content

Commit

Permalink
Merge branch 'f-#109-lookup-error'. Closes #109.
Browse files Browse the repository at this point in the history
- `$` only warns and also performs partial matching if column not found.
- `[[` returns `NULL` if column not found.
  • Loading branch information
Kirill Müller committed Jun 30, 2016
2 parents cea29b6 + 4c3dca3 commit 79cffa3
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 12 deletions.
10 changes: 4 additions & 6 deletions R/tbl-df.r
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ print.tbl_df <- function(x, ..., n = NULL, width = NULL) {
colname <- i
else
colname <- j
if (is.character(colname) && length(colname) == 1L && !(colname %in% names(x))) {
stopc("Unknown column '", colname, "'")
}
if (!exact) {
warningc("exact ignored")
}
Expand All @@ -34,10 +31,11 @@ print.tbl_df <- function(x, ..., n = NULL, width = NULL) {
#' @export
`$.tbl_df` <- function(x, i) {
if (is.character(i) && !(i %in% names(x))) {
stopc("Unknown column '", i, "'")
warningc("Unknown column '", i, "'")
.subset2(x, i, exact = FALSE)

This comment has been minimized.

Copy link
@hadley

hadley Jun 30, 2016

Member

I think this should just return NULL to make it really clear what's happening. I'm happy to not support partial matching here

} else {
.subset2(x, i)
}

.subset2(x, i)
}

#' @export
Expand Down
15 changes: 9 additions & 6 deletions tests/testthat/test-tbl-df.R
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ test_that("can use recursive indexing with [[", {
expect_equal(foo[[c("x", "y")]], 1:3)
})

test_that("[[ throws error if name doesn't exist", {
test_that("[[ returns NULL if name doesn't exist", {
df <- data_frame(x = 1)
expect_error(df[["y"]], "Unknown column 'y'")
expect_error(df[[1, "y"]], "Unknown column 'y'")
expect_null(df[["y"]])
expect_null(df[[1, "y"]])
})

test_that("can use two-dimensional indexing with [[", {
Expand All @@ -126,13 +126,16 @@ test_that("can use two-dimensional indexing with [[", {

test_that("$ throws error if name doesn't exist", {
df <- data_frame(x = 1)
expect_error(df$y, "Unknown column 'y'")
expect_warning(expect_null(df$y),
"Unknown column 'y'")
})

test_that("$ doesn't do partial matching", {
df <- data_frame(partial = 1)
expect_error(df$p, "Unknown column 'p'")
expect_error(df$part, "Unknown column 'part'")
expect_warning(expect_identical(df$p, 1),
"Unknown column 'p'")
expect_warning(expect_identical(df$part, 1),
"Unknown column 'part'")
expect_error(df$partial, NA)
})

Expand Down

0 comments on commit 79cffa3

Please sign in to comment.