Deprecate options(dplyr.legacy_locale =) usage
#7766
Conversation
| # This deprecation is a bit special. Since it is a global option that only the | ||
| # end user would ever set, we set `user_env = globalenv()` so that it always | ||
| # looks like a "direct" usage to lifecycle. This also makes our lives easier, | ||
| # because `user_env` would have to be threaded all the way up through the | ||
| # exported `grouped_df()` function, which is then used in many places | ||
| # throughout dplyr. Additionally, we've bypassed `deprecate_soft()` and gone | ||
| # straight to `deprecate_warn()` since this is only an end user facing option. | ||
| lifecycle::deprecate_warn( | ||
| when = "1.2.0", | ||
| what = I("`options(dplyr.legacy_locale =)`"), | ||
| details = c( | ||
| i = "If needed for `arrange()`, use `arrange(.locale =)` instead.", | ||
| i = "If needed for `group_by() |> summarise()`, follow up with an additional `arrange(.locale =)` call." | ||
| ), | ||
| user_env = globalenv() | ||
| ) |
There was a problem hiding this comment.
This is the most important paragraph to read, because this deprecation process is very special since it is a global option that is only used by end users:
- We force
user_env = globalenv()to make usage always look "direct" to lifecycle (because an end user "directly" set the global option) - We start with once per session
deprecate_warn()rather thandeprecate_soft()(that wouldn't really mean much anyways sinceuser_env = globalenv())
I started off with deprecate_soft() and tried to thread user_env up the stack, but it's quite a nightmare. It gets pushed up through grouped_df() which is used all over the place, way more than just group_by().
Instead, this seems like a much saner option, especially since it is a global option that should only ever be set by end users anyway (a quick glance at the cran github org confirms that no R package is setting this for users).
There was a problem hiding this comment.
Based on https://github.com/search?q=org%3Acran%20dplyr.legacy_locale&type=code, this is only going to affect user code, not CRAN packages, so this behaviour sounds reasonable to me.
hadley
left a comment
hadley
left a comment
There was a problem hiding this comment.
My only request is to be a bit more concrete about exactly what you need to do to restore legacy behaviour.
| # This deprecation is a bit special. Since it is a global option that only the | ||
| # end user would ever set, we set `user_env = globalenv()` so that it always | ||
| # looks like a "direct" usage to lifecycle. This also makes our lives easier, | ||
| # because `user_env` would have to be threaded all the way up through the | ||
| # exported `grouped_df()` function, which is then used in many places | ||
| # throughout dplyr. Additionally, we've bypassed `deprecate_soft()` and gone | ||
| # straight to `deprecate_warn()` since this is only an end user facing option. | ||
| lifecycle::deprecate_warn( | ||
| when = "1.2.0", | ||
| what = I("`options(dplyr.legacy_locale =)`"), | ||
| details = c( | ||
| i = "If needed for `arrange()`, use `arrange(.locale =)` instead.", | ||
| i = "If needed for `group_by() |> summarise()`, follow up with an additional `arrange(.locale =)` call." | ||
| ), | ||
| user_env = globalenv() | ||
| ) |
There was a problem hiding this comment.
Based on https://github.com/search?q=org%3Acran%20dplyr.legacy_locale&type=code, this is only going to affect user code, not CRAN packages, so this behaviour sounds reasonable to me.
Closes #7760
This is what caused both
arrange()andgroup_by() |> summarise()to fall back to the old system-locale ordering, rather than using the new C locale approach and requiringarrange(.locale =)instead.It was always meant to be a temporary global option that people could slap at the top of their scripts if they just didn't want to deal with it right now.