Skip to content

Commit

Permalink
Switch to dplyr.legacy_locale
Browse files Browse the repository at this point in the history
  • Loading branch information
DavisVaughan committed Jul 11, 2022
1 parent 390fa80 commit a71ee8d
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 37 deletions.
8 changes: 4 additions & 4 deletions R/group-by.r
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@
#'
#' Prior to dplyr 1.1.0, character vector grouping columns were ordered in the
#' system locale. If you need to temporarily revert to this behavior, you can
#' set the global option `dplyr.legacy_group_by_locale` to `TRUE`, but this
#' should be used sparingly and you should expect this option to be removed in a
#' future version of dplyr. It is better to update existing code to explicitly
#' call `arrange()` instead.
#' set the global option `dplyr.legacy_locale` to `TRUE`, but this should be
#' used sparingly and you should expect this option to be removed in a future
#' version of dplyr. It is better to update existing code to explicitly call
#' `arrange()` instead.
#'
#' @export
#' @examples
Expand Down
15 changes: 1 addition & 14 deletions R/grouped-df.r
Original file line number Diff line number Diff line change
Expand Up @@ -298,26 +298,13 @@ dplyr_locate_sorted_groups <- function(x) {
out <- vec_locate_sorted_groups(x, nan_distinct = TRUE)
out$loc <- new_list_of(out$loc, ptype = integer())

if (dplyr_legacy_group_by_locale()) {
if (dplyr_legacy_locale()) {
# Temporary legacy support for respecting the system locale
out <- vec_slice(out, vec_order_base(out$key))
}

out
}
dplyr_legacy_group_by_locale <- function() {
out <- peek_option("dplyr.legacy_group_by_locale") %||% FALSE

if (!is_bool(out)) {
message <- paste0(
"Global option `dplyr.legacy_group_by_locale` ",
"must be a single `TRUE` or `FALSE`."
)
abort(message, call = NULL)
}

out
}

group_intersect <- function(x, new) {
intersect(group_vars(x), names(new))
Expand Down
16 changes: 16 additions & 0 deletions R/locale.R
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,19 @@ dplyr_locale <- function() {
has_minimum_stringi <- function() {
is_installed("stringi", version = "1.5.3")
}

dplyr_legacy_locale <- function() {
# Used to determine if `group_by()` and `arrange()` should use
# base R's `order()` for sorting, which respects the system locale and was
# our sorting engine pre-1.1.0.
out <- peek_option("dplyr.legacy_locale") %||% FALSE

if (!is_bool(out)) {
abort(
"Global option `dplyr.legacy_locale` must be a single `TRUE` or `FALSE`.",
call = NULL
)
}

out
}
8 changes: 4 additions & 4 deletions man/group_by.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions tests/testthat/_snaps/grouped-df.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,3 @@
Error in `grouped_df()`:
! `vars` must be a character vector.

# `dplyr.legacy_group_by_locale` is validated

Code
dplyr_legacy_group_by_locale()
Condition
Error:
! Global option `dplyr.legacy_group_by_locale` must be a single `TRUE` or `FALSE`.

8 changes: 8 additions & 0 deletions tests/testthat/_snaps/locale.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,11 @@
Error in `dplyr_locale()`:
! If set, the global option `dplyr.locale` must be a string.

# `dplyr_legacy_locale()` respects `dplyr.legacy_locale`

Code
dplyr_legacy_locale()
Condition
Error:
! Global option `dplyr.legacy_locale` must be a single `TRUE` or `FALSE`.

9 changes: 2 additions & 7 deletions tests/testthat/test-grouped-df.r
Original file line number Diff line number Diff line change
Expand Up @@ -202,18 +202,13 @@ test_that("groups are ordered in the C locale", {
expect_identical(result$x, c("A", "Z", "a", "b"))
})

test_that("using the global option `dplyr.legacy_group_by_locale` forces the system locale", {
test_that("using the global option `dplyr.legacy_locale` forces the system locale", {
skip_if_not(has_collate_locale("en_US"), message = "Can't use 'en_US' locale")

local_options(dplyr.legacy_group_by_locale = TRUE)
local_options(dplyr.legacy_locale = TRUE)
withr::local_collate("en_US")

df <- tibble(x = c("a", "A", "Z", "b"))
result <- compute_groups(df, "x")
expect_identical(result$x, c("a", "A", "b", "Z"))
})

test_that("`dplyr.legacy_group_by_locale` is validated", {
local_options(dplyr.legacy_group_by_locale = 1)
expect_snapshot(error = TRUE, dplyr_legacy_group_by_locale())
})
14 changes: 14 additions & 0 deletions tests/testthat/test-locale.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,17 @@ test_that("`dplyr_locale()` respects `dplyr.locale`", {
dplyr_locale()
})
})

test_that("`dplyr_legacy_locale()` is `FALSE` by default", {
expect_false(dplyr_legacy_locale())
})

test_that("`dplyr_legacy_locale()` respects `dplyr.legacy_locale`", {
local_options(dplyr.legacy_locale = TRUE)
expect_true(dplyr_legacy_locale())

local_options(dplyr.legacy_locale = 1)
expect_snapshot(error = TRUE, {
dplyr_legacy_locale()
})
})

0 comments on commit a71ee8d

Please sign in to comment.