Skip to content

Commit

Permalink
Update tests for 1-based indices
Browse files Browse the repository at this point in the history
  • Loading branch information
@romainfrancois
romainfrancois committed May 28, 2018
1 parent d06199d commit a5929e4
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 11 deletions.
2 changes: 1 addition & 1 deletion tests/testthat/test-arrange.r
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ test_that("arrange keeps the grouping structure (#605)", {
res <- dat %>% group_by(g) %>% arrange(x)
expect_is(res, "grouped_df")
expect_equal(res$x, 1:4)
expect_equal(group_rows(res), list(c(1, 3), c(0, 2)))
expect_equal(group_rows(res), list(c(2, 4), c(1, 3)))
})

test_that("arrange handles complex vectors", {
Expand Down
12 changes: 6 additions & 6 deletions tests/testthat/test-group_data.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,27 @@ context("group_data")

test_that("group_rows works for 3 most important subclasses (#3489)", {
df <- data.frame(x=c(1,1,2,2))
expect_equal(group_rows(df), list(0:3))
expect_equal(group_rows(group_by(df,x)), list(0:1, 2:3))
expect_equal(group_rows(rowwise(df)), as.list(0:3))
expect_equal(group_rows(df), list(1:4))
expect_equal(group_rows(group_by(df,x)), list(1:2, 3:4))
expect_equal(group_rows(rowwise(df)), as.list(1:4))
})

test_that("group_data returns a tidy tibble (#3489)", {
df <- tibble(x = c(1,1,2,2))

expect_identical(
group_data(df),
tibble(.rows=list(0:3))
tibble(.rows=list(1:4))
)

expect_identical(
group_by(df,x) %>% group_data(),
tibble(x = c(1,2), .rows = list(0:1, 2:3))
tibble(x = c(1,2), .rows = list(1:2, 3:4))
)

expect_identical(
rowwise(df) %>% group_data(),
tibble(.rows = as.list(0:3))
tibble(.rows = as.list(1:4))
)
})

Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-sets.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ test_that("set operations reconstruct grouping metadata (#3587)", {
df1 <- tibble(x = 1:4, g = rep(1:2, each = 2)) %>% group_by(g)
df2 <- tibble(x = 3:6, g = rep(2:3, each = 2))

expect_equal( setdiff(df1, df2) %>% group_rows(), list(0:1))
expect_equal( intersect(df1, df2) %>% group_rows(), list(0:1))
expect_equal( union(df1, df2) %>% group_rows(), list(4:5, 2:3, 0:1))
expect_equal( setdiff(df1, df2) %>% group_rows(), list(1:2))
expect_equal( intersect(df1, df2) %>% group_rows(), list(1:2))
expect_equal( union(df1, df2) %>% group_rows(), list(5:6, 3:4, 1:2))

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Sep 19, 2018

Member

@romainfrancois: This test now works differently on Windows (returning list(1:2, 3:4, 5:6)). Any idea why? Do we guarantee order in union() and the other set operations?

This comment has been minimized.

Sign in to view

Copy link
@romainfrancois

romainfrancois Sep 19, 2018

Author Member

I don't think that we do

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Sep 19, 2018

Member

We can adapt the test then. Still, I'm slightly worried why the behavior is different across OSs (or compilers?).

This comment has been minimized.

Sign in to view

Copy link
@romainfrancois

romainfrancois Sep 19, 2018

Author Member

boost might hash things differently on different platforms. I suppose this won't be a problem once we can use hashing from vctrs

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Sep 19, 2018

Member

full_join() keeps the order, though, consistently on both platforms. Perhaps we could get rid of some C++ code if we forward union() to full_join() (and setdiff() to anti_join() and intersect() to semi_join() ) ?

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Sep 19, 2018

Member

But then a join might be more expensive...

This comment has been minimized.

Sign in to view

Copy link
@romainfrancois

romainfrancois Sep 19, 2018

Author Member

And we could do it from the R side: less code. can you open an issue for that please

This comment has been minimized.

Sign in to view

Copy link
@krlmlr

krlmlr Sep 19, 2018

Member

Not quite:

library(tidyverse)

setdiff(c(1, 1), 2)
#> [1] 1
setdiff(tibble(a = c(1, 1)), tibble(a = 2))
#> # A tibble: 1 x 1
#>       a
#>   <dbl>
#> 1     1

intersect(c(1, 1), 1)
#> [1] 1
intersect(tibble(a = c(1, 1)), tibble(a = 1))
#> # A tibble: 1 x 1
#>       a
#>   <dbl>
#> 1     1

union(c(1, 1), 1)
#> [1] 1
union(tibble(a = c(1, 1)), tibble(a = 1))
#> # A tibble: 1 x 1
#>       a
#>   <dbl>
#> 1     1

Created on 2018-09-19 by the reprex package (v0.2.1)

I'll fix the order in union(), about to submit a PR for that.
})
2 changes: 1 addition & 1 deletion tests/testthat/test-slice.r
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ test_that("slice works fine if n > nrow(df) (#1269)", {
test_that("slice strips grouped indices (#1405)", {
res <- mtcars %>% group_by(cyl) %>% slice(1) %>% mutate(mpgplus = mpg + 1)
expect_equal(nrow(res), 3L)
expect_equal(group_rows(res), as.list(0:2))
expect_equal(group_rows(res), as.list(1:3))
})

test_that("slice works with zero-column data frames (#2490)", {
Expand Down

0 comments on commit a5929e4

Please sign in to comment.