-
Notifications
You must be signed in to change notification settings - Fork 282
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
First attempt to adapt pivot_longer to sf #1151
Conversation
as pivot_longer is not yet a s3_methods, the new function is only available under the name pivot_longer.sf
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it is a better idea to get confirmation from the authors that pivot_longer
is never going to be a generic (and hear why), as opposed to e.g. gather
, before we go along with suggesting users to write pivot_longer.sf(...)
.
#' @examples | ||
#' library(tidyr) | ||
#' nc %>% select(SID74, SID79) %>% gather("VAR", "SID", -geometry) %>% summary() | ||
pivot_longer.sf <- function(data, cols, names_to = "name", names_prefix = NULL, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to replace all the parameters that you want to pass on untouched as ... , unless you want to change their default. This would spare you (and me) the work of documenting them, and keeping track of changes in upcoming tidyr versions.
@@ -405,6 +433,7 @@ register_all_s3_methods = function() { | |||
register_s3_method("dplyr", "ungroup", "sf") | |||
register_s3_method("tidyr", "gather", "sf") | |||
register_s3_method("tidyr", "spread", "sf") | |||
register_s3_method("tidyr", "pivot_longer", "sf") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if pivot_longer
is not an s3 method, this shouldn't work.
I agree with your two comments. I will wait until the situation around
pivot_longer is clearer. In between, I will prepare everything for all the
pivot_* functions.
…On Sun, Sep 15, 2019 at 6:33 PM Edzer Pebesma ***@***.***> wrote:
***@***.**** commented on this pull request.
Maybe it is a better idea to get confirmation from the authors that
pivot_longer is never going to be a generic (and hear why), as opposed to
e.g. gather, before we go along with suggesting users to write
pivot_longer.sf(...).
------------------------------
In R/tidyverse.R
<#1151 (comment)>:
> @@ -249,6 +249,34 @@ spread.sf <- function(data, key, value, fill = NA, convert = FALSE, drop = TRUE,
drop = drop, sep = sep), sf_column_name = attr(data, "sf_column"))
}
+#' @name tidyverse
+#' @export
+#' @examples
+#' library(tidyr)
+#' nc %>% select(SID74, SID79) %>% gather("VAR", "SID", -geometry) %>% summary()
+pivot_longer.sf <- function(data, cols, names_to = "name", names_prefix = NULL,
I suggest to replace all the parameters that you want to pass on untouched
as ... , unless you want to change their default. This would spare you (and
me) the work of documenting them, and keeping track of changes in upcoming
tidyr versions.
------------------------------
In R/tidyverse.R
<#1151 (comment)>:
> @@ -405,6 +433,7 @@ register_all_s3_methods = function() {
register_s3_method("dplyr", "ungroup", "sf")
register_s3_method("tidyr", "gather", "sf")
register_s3_method("tidyr", "spread", "sf")
+ register_s3_method("tidyr", "pivot_longer", "sf")
if pivot_longer is not an s3 method, this shouldn't work.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1151?email_source=notifications&email_token=AB4BD2IV2JSPCENW3NU4HNLQJZPW3A5CNFSM4IW2RIJ2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCEYDF7Q#pullrequestreview-288371454>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB4BD2PYAR2SKTFHG435VG3QJZPW3ANCNFSM4IW2RIJQ>
.
|
please re-PR when there is some progress in this area. |
As pivot_longer is not yet a s3_methods, the new function is only available under the name pivot_longer.sf...