Skip to content

Commit

Permalink
Fix performance issue in vec_order() with classed vectors (#1197)
Browse files Browse the repository at this point in the history
Closes tidyverse/dplyr#5423
Introduced in #1142
  • Loading branch information
lionel- authored Jul 23, 2020
1 parent bcfd66f commit 8855bdd
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 0 deletions.
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@

# vctrs (development version)

* Fixed performance issue with `vec_order()` on classed vectors which
affected `dplyr::group_by()` (tidyverse/dplyr#5423).

* `vec_set_names()` no longer alters the input in-place (#1194).

* New `vec_proxy_order()` that provides an ordering proxy for use in
Expand Down
3 changes: 3 additions & 0 deletions R/compare.R
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ order_proxy <- function(proxy, direction = "asc", na_value = "largest") {
})
exec("order", !!!args, decreasing = decreasing, na.last = na.last)
} else if (is_character(proxy) || is_logical(proxy) || is_integer(proxy) || is_double(proxy)) {
if (is.object(proxy)) {
proxy <- unstructure(proxy)
}
order(proxy, decreasing = decreasing, na.last = na.last)
} else {
abort("Invalid type returned by `vec_proxy_compare()`.")
Expand Down
6 changes: 6 additions & 0 deletions tests/testthat/test-compare.R
Original file line number Diff line number Diff line change
Expand Up @@ -337,3 +337,9 @@ test_that("can order data frames (and subclasses) with matrix columns", {
df$x <- tibble::tibble(y = matrix(1:2, 2))
expect_identical(vec_order(df), 1:2)
})

test_that("classed proxies do not affect performance (tidyverse/dplyr#5423)", {
skip_on_cran()
x <- glue::glue("{1:10000}")
expect_time_lt(vec_order(x), 0.2)
})

0 comments on commit 8855bdd

Please sign in to comment.