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

all.equal.tbl_df throws an error when tbl_df objects have columns of mode "list" #107

Closed
BillDunlap opened this issue Jun 22, 2016 · 4 comments
Closed
Milestone

Comments

@BillDunlap
Copy link

@BillDunlap BillDunlap commented Jun 22, 2016

There is a problem in coming up with a canonical row order for tbl_df's containing lists,
at least if the list is in the first column. It would be simplest if base::order() did something
reasonable for lists.

T <- tibble::data_frame(A=list(1,c("b","a"),c(pi,exp(1),-1)), B=13:11)
all.equal(T, T) # expect TRUE
Error in (function (..., na.last = TRUE, decreasing = FALSE, method = c("shell", :
unimplemented type 'list' in 'listgreater'
all.equal(T, T, ignore_row_order = FALSE) # expect TRUE
[1] TRUE
packageVersion("tibble")
[1] ‘1.0.12’
version$version.string
[1] "R version 3.3.0 (2016-05-03)"

While you are messing around with this, it would be good to use the idiom
do.call(order, unname(frame))
instead
do.call(order, frame)
as the latter gives an error if the frame has a column with the name of an argument
to order. E.g.,

U <- data_frame(method=c("old","new"), time=c(4.5, 2.3))
all.equal(U, U)
Error in match.arg(method) : 'arg' must be of length 1

The following code in the ignore_row_order part of all.equal.tbl_df
fixes both problems. I don't know if there are any other unorderable
types allowed as columns of tbl_df objects.

if (ignore_row_order) {
    patch_for_order <- function(tbl_df) {
        for (i in seq_len(ncol(tbl_df))) {
            if (is.list(tbl_df[[i]])) {
                tbl_df[[i]] <- paste(collapse = "\n", deparse(tbl_df[[i]]))
            }
        }
        unname(tbl_df)
    }
    target <- target[do.call(order, patch_for_order(target)), ]
    current <- current[do.call(order, patch_for_order(current)), ]
}
@gaborcsardi
Copy link
Member

@gaborcsardi gaborcsardi commented Jul 24, 2016

Yes, I am running into this, too. You just can't call all.equal on tibbles with list columns. Which is a pity because testthat::expect_equal also calls all.equal.

A sloppy workaround is sg like expect_equal(as.data.frame(x), as.data.frame(y)) && expect_equal(class(x), class(y)).

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Jul 30, 2016

If dplyr is not loaded, you can use

expect_equal(x, y, ignore_row_order = FALSE)

I'd rather not support ignore_row_order = TRUE for non-atomic columns; your example should throw an error instead.

The situation is worse when dplyr is loaded: It overrides all.equal.tbl_df() and always uses a join to check equality, so it doesn't even work with ignore_row_order = FALSE. If this poses a problem, please file an issue there.

krlmlr pushed a commit that referenced this issue Jul 30, 2016
- `all.equal()` doesn't throw an error anymore if one of the columns is named `na.last`, `decreasing` or `method` (#107, @BillDunlap).
krlmlr pushed a commit that referenced this issue Jul 30, 2016
- `all.equal()` doesn't throw an error anymore if one of the columns is named `na.last`, `decreasing` or `method` (#107, @BillDunlap).
@krlmlr krlmlr added the ready label Jul 30, 2016
@krlmlr krlmlr added this to the 2.0 milestone Aug 8, 2016
@krlmlr krlmlr added this to the 2.0 milestone Aug 8, 2016
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Aug 16, 2016

Closing for now.

@krlmlr krlmlr closed this Aug 16, 2016
@krlmlr krlmlr removed the ready label Aug 16, 2016
krlmlr added a commit that referenced this issue Aug 26, 2016
- The `tibble.width` option is used for `glimpse()` only if it is finite (#153, @kwstat).
- New `as_tibble.poly()` to support conversion of a `poly` object to a tibble (#110).
- `add_row()` now correctly handles existing columns of type `list` that are not updated (#148).
- `all.equal()` doesn't throw an error anymore if one of the columns is named `na.last`, `decreasing` or `method` (#107, @BillDunlap).

- New `add_column()`, analogously to `add_row()` (#99).
- `print.tbl_df()` gains `n_extra` method and will have the same interface as `trunc_mat()` from now on.
- `add_row()` and `add_column()` gain `.before` and `.after` arguments which indicate the row (by number) or column (by number or name) before or after which the new data are inserted. Updated or added columns cannot be named `.before` or `.after` (#99).
- Rename `frame_data()` to `tribble()`, stands for "transposed tibble". The former is still available as alias (#132, #143).

- `add_row()` now can add multiple rows, with recycling (#142, @jennybc).
- Use multiply character `×` instead of `x` when printing dimensions (#126). Output tests had to be disabled for this on Windows.
- Back-tick non-semantic column names on output (#131).
- Use `dttm` instead of `time` for `POSIXt` values (#133), which is now used for columns of the `difftime` class.
- Better output for 0-row results when total number of rows is unknown (e.g., for SQL data sources).

- New object summary vignette that shows which methods to define for custom vector classes to be used as tibble columns (#151).
- Added more examples for `print.tbl_df()`, now using data from `nycflights13` instead of `Lahman` (#121), with guidance to install `nycflights13` package if necessary (#152).
- Minor changes in vignette (#115, @helix123).
@github-actions
Copy link

@github-actions github-actions bot commented Dec 14, 2020

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants