Skip to content

Commit

Permalink
Revert "warn if converting data frame with row names to tibble"
Browse files Browse the repository at this point in the history
This reverts commit 63b68f3.

No point in doing this:

- dplyr tests fail massively
- users won't like it
- no harm done by keeping row names
  • Loading branch information
Kirill Müller committed May 11, 2016
1 parent fd576cb commit 85a62b7
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 49 deletions.
6 changes: 0 additions & 6 deletions R/dataframe.R
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@ as_data_frame.tbl_df <- function(x, ...) {
#' @export
#' @rdname as_data_frame
as_data_frame.data.frame <- function(x, ...) {
warn_if_has_rownames(x)
class(x) <- c("tbl_df", "tbl", "data.frame")
x
}
Expand Down Expand Up @@ -313,8 +312,3 @@ invalid_df <- function(problem, df, vars) {
)
}

warn_if_has_rownames <- function(df) {
if (has_rownames(df)) {
warning("Creating a tibble with row names will be deprecated. Remove or convert to a column.", call. = FALSE)
}
}
2 changes: 0 additions & 2 deletions tests/testthat/helper-data.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,3 @@ df_all <- data_frame(
g = as.POSIXct("2015-12-09 10:51:34 UTC") + c(1:2, NA),
h = as.list(c(1:2, NA))
)

mtcars2 <- remove_rownames(mtcars)
24 changes: 12 additions & 12 deletions tests/testthat/test-equality.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
context("Equality")

test_that("data frames equal to themselves", {
expect_true(all.equal(as_data_frame(mtcars2), as_data_frame(mtcars2)))
expect_true(all.equal(as_data_frame(mtcars), as_data_frame(mtcars)))
expect_true(all.equal(as_data_frame(iris), as_data_frame(iris)))
expect_true(all.equal(as_data_frame(df_all), as_data_frame(df_all)))
})
Expand All @@ -11,23 +11,23 @@ test_that("data frames equal to random permutations of themselves", {
x[sample(nrow(x)), sample(ncol(x)), drop = FALSE]
}

expect_equal(as_data_frame(mtcars2), scramble(as_data_frame(mtcars2)))
expect_equal(as_data_frame(iris), scramble(as_data_frame(iris)))
expect_equal(as_data_frame(df_all), scramble(as_data_frame(df_all)))
expect_equal(as_data_frame(mtcars), as_data_frame(scramble(mtcars)))
expect_equal(as_data_frame(iris), as_data_frame(scramble(iris)))
expect_equal(as_data_frame(df_all), as_data_frame(scramble(df_all)))
})

test_that("data frames not equal if missing row", {
expect_match(all.equal(as_data_frame(mtcars2), as_data_frame(mtcars2)[-1, ]), "Different number of rows")
expect_match(all.equal(as_data_frame(iris), as_data_frame(iris)[-1, ]), "Different number of rows")
expect_match(all.equal(as_data_frame(df_all), as_data_frame(df_all)[-1, ]), "Different number of rows")
expect_match(all.equal(as_data_frame(mtcars), as_data_frame(mtcars)[-1, ]), "Different number of rows")
expect_match(all.equal(as_data_frame(iris), as_data_frame(iris)[-1, ]), "Different number of rows")
expect_match(all.equal(as_data_frame(df_all), as_data_frame(df_all)[-1, ]), "Different number of rows")
})

test_that("data frames not equal if missing col", {
expect_match(all.equal(as_data_frame(mtcars2), as_data_frame(mtcars2)[, -1]), "Cols in x but not y: mpg")
expect_match(all.equal(as_data_frame(mtcars2)[, -1], as_data_frame(mtcars2)), "Cols in y but not x: mpg")
expect_match(all.equal(as_data_frame(iris), as_data_frame(iris)[, -1]), "Cols in x but not y: Sepal.Length")
expect_match(all.equal(as_data_frame(df_all), as_data_frame(df_all)[, -1]), "Cols in x but not y: a")
expect_match(all.equal(as_data_frame(mtcars2), rev(as_data_frame(mtcars2)),
expect_match(all.equal(as_data_frame(mtcars), as_data_frame(mtcars)[, -1]), "Cols in x but not y: mpg")
expect_match(all.equal(as_data_frame(mtcars)[, -1], as_data_frame(mtcars)), "Cols in y but not x: mpg")
expect_match(all.equal(as_data_frame(iris), as_data_frame(iris)[, -1]), "Cols in x but not y: Sepal.Length")
expect_match(all.equal(as_data_frame(df_all), as_data_frame(df_all)[, -1]), "Cols in x but not y: a")
expect_match(all.equal(as_data_frame(mtcars), rev(as_data_frame(mtcars)),
ignore_col_order = FALSE),
"Column names same but in different order")
})
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-glimpse.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ context("Glimpse")

test_that("glimpse output matches known output", {
expect_output_file_rel(
glimpse(as_data_frame(mtcars2), width = 70L),
glimpse(as_data_frame(mtcars), width = 70L),
"glimpse/mtcars-70.txt")

expect_output_file_rel(
Expand Down
25 changes: 8 additions & 17 deletions tests/testthat/test-rownames.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,36 +8,27 @@ test_that("has_rownames and remove_rownames", {
expect_false(has_rownames(1:10))
})

test_that("creating a tibble with row names raises a warning", {
expect_warning(as_data_frame(mtcars), "deprecated")
expect_warning(as_data_frame(mtcars2), NA)
})

test_that("setting row names on a tibble raises a warning", {
mtcars_tbl <- as_data_frame(mtcars2)
mtcars_tbl <- as_data_frame(mtcars)
expect_warning(rownames(mtcars_tbl) <- rownames(mtcars), "deprecated")
})

test_that("rownames_to_column keeps the tbl classes (#882)", {
expect_true(has_rownames(mtcars))
res <- rownames_to_column(mtcars, "Make&Model")
res <- rownames_to_column( mtcars, "Make&Model" )
expect_false(has_rownames(res))
expect_equal(class(res), class(mtcars))
expect_equal( class(res), class(mtcars) )
expect_error(rownames_to_column( mtcars, "wt"),
paste("There is a column named wt already!"))

expect_warning(mtcars1 <- as_data_frame(mtcars))
expect_true(has_rownames(mtcars1))
res1 <- rownames_to_column(mtcars1, "Make&Model")
paste("There is a column named wt already!") )
res1 <- rownames_to_column( as_data_frame(mtcars), "Make&Model" )
expect_false(has_rownames(res1))
expect_equal( class(res1), class(mtcars1))
expect_equal( class(res1), class(as_data_frame(mtcars)) )
expect_error(rownames_to_column( mtcars, "wt"),
paste("There is a column named wt already!"))
paste("There is a column named wt already!") )
})

test_that("column_to_rownames returns tbl", {
var <- "car"
expect_warning(mtcars1 <- as_data_frame(mtcars))
mtcars1 <- as_data_frame(mtcars)
expect_true(has_rownames(mtcars1))
res0 <- rownames_to_column(mtcars1, var)
expect_warning(res <- column_to_rownames(res0, var))
Expand Down
18 changes: 9 additions & 9 deletions tests/testthat/test-tbl-df.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,27 @@ context("tbl_df")
# [ -----------------------------------------------------------------------

test_that("[ never drops", {
mtcars2 <- as_data_frame(mtcars2)
mtcars2 <- as_data_frame(mtcars)
expect_is(mtcars2[, 1], "data.frame")
expect_is(mtcars2[, 1], "tbl_df")
expect_equal(mtcars2[, 1], mtcars2[1])
})

test_that("[ retains class", {
mtcars2 <- as_data_frame(mtcars2)
mtcars2 <- as_data_frame(mtcars)
expect_identical(class(mtcars2), class(mtcars2[1:5, ]))
expect_identical(class(mtcars2), class(mtcars2[, 1:5]))
expect_identical(class(mtcars2), class(mtcars2[1:5, 1:5]))
})

test_that("[ and as_data_frame commute", {
mtcars2 <- as_data_frame(mtcars2)
expect_identical(mtcars2, as_data_frame(mtcars2))
expect_identical(mtcars2[], as_data_frame(mtcars2[]))
expect_identical(mtcars2[1:5, ], as_data_frame(mtcars2[1:5, ]))
expect_identical(mtcars2[, 1:5], as_data_frame(mtcars2[, 1:5]))
expect_identical(mtcars2[1:5, 1:5], as_data_frame(mtcars2[1:5, 1:5]))
expect_identical(mtcars2[1:5], as_data_frame(mtcars2[1:5]))
mtcars2 <- as_data_frame(mtcars)
expect_identical(mtcars2, as_data_frame(mtcars))
expect_identical(mtcars2[], remove_rownames(as_data_frame(mtcars[])))
expect_identical(mtcars2[1:5, ], remove_rownames(as_data_frame(mtcars[1:5, ])))
expect_identical(mtcars2[, 1:5], remove_rownames(as_data_frame(mtcars[, 1:5])))
expect_identical(mtcars2[1:5, 1:5], remove_rownames(as_data_frame(mtcars[1:5, 1:5])))
expect_identical(mtcars2[1:5], remove_rownames(as_data_frame(mtcars[1:5])))
})

test_that("[ with 0 cols creates correct row names (#656)", {
Expand Down
3 changes: 1 addition & 2 deletions tests/testthat/test-trunc-mat.R
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
context("Truncated matrix")

test_that("trunc_mat output matches known output", {
expect_warning(mtcars1 <- as_data_frame(mtcars))
expect_output_file_rel(
print(mtcars1, n = 8L, width = 30L),
print(as_data_frame(mtcars), n = 8L, width = 30L),
"trunc_mat/mtcars-8-30.txt")

expect_output_file_rel(
Expand Down

0 comments on commit 85a62b7

Please sign in to comment.