search

nathaneastwood / poorman

A poor man's dependency free grammar of data manipulation
https://nathaneastwood.github.io/poorman/
Other
338 stars 15 forks source link

Rework arrange to work with different input types #69

Closed alex-gable closed 3 years ago

alex-gable commented 3 years ago

Describe the bug Arranging using a character vector of column names produces output which differs from dplyr in three different cases.

To Reproduce

Ungrouped ``` r `%>%` <- poorman::`%>%` arrange_vars <- c("carb", "am") # JUST ARRANGE mtcars %>% dplyr::arrange(arrange_vars) %>% head() #> Error: arrange() failed at implicit mutate() step. #> ✖ Could not create a temporary column for `..1`. #> ℹ `..1` is `arrange_vars`. mtcars %>% poorman::arrange(arrange_vars) %>% head() #> mpg cyl disp hp drat wt qsec vs am gear carb #> Mazda RX4 Wag 21 6 160 110 3.9 2.875 17.02 0 1 4 4 #> Mazda RX4 21 6 160 110 3.9 2.620 16.46 0 1 4 4 # ARRANGE USING ACROSS (WORKS BUT MESSAGES) mtcars %>% dplyr::arrange(dplyr::across(arrange_vars)) %>% head() #> Note: Using an external vector in selections is ambiguous. #> ℹ Use `all_of(arrange_vars)` instead of `arrange_vars` to silence this message. #> ℹ See . #> This message is displayed once per session. #> mpg cyl disp hp drat wt qsec vs am gear carb #> Hornet 4 Drive 21.4 6 258.0 110 3.08 3.215 19.44 1 0 3 1 #> Valiant 18.1 6 225.0 105 2.76 3.460 20.22 1 0 3 1 #> Toyota Corona 21.5 4 120.1 97 3.70 2.465 20.01 1 0 3 1 #> Datsun 710 22.8 4 108.0 93 3.85 2.320 18.61 1 1 4 1 #> Fiat 128 32.4 4 78.7 66 4.08 2.200 19.47 1 1 4 1 #> Toyota Corolla 33.9 4 71.1 65 4.22 1.835 19.90 1 1 4 1 mtcars %>% poorman::arrange(poorman::across((arrange_vars))) %>% head() #> mpg cyl disp hp drat wt qsec vs am gear carb #> NA NA NA NA NA NA NA NA NA NA NA NA #> NA.1 NA NA NA NA NA NA NA NA NA NA NA #> NA.2 NA NA NA NA NA NA NA NA NA NA NA #> NA.3 NA NA NA NA NA NA NA NA NA NA NA #> NA.4 NA NA NA NA NA NA NA NA NA NA NA #> NA.5 NA NA NA NA NA NA NA NA NA NA NA # ARRANGE "CORRECTLY" USING ACROSS WITH TIDYSELECT-ER mtcars %>% dplyr::arrange(dplyr::across(tidyselect::all_of(arrange_vars))) %>% head() #> mpg cyl disp hp drat wt qsec vs am gear carb #> Hornet 4 Drive 21.4 6 258.0 110 3.08 3.215 19.44 1 0 3 1 #> Valiant 18.1 6 225.0 105 2.76 3.460 20.22 1 0 3 1 #> Toyota Corona 21.5 4 120.1 97 3.70 2.465 20.01 1 0 3 1 #> Datsun 710 22.8 4 108.0 93 3.85 2.320 18.61 1 1 4 1 #> Fiat 128 32.4 4 78.7 66 4.08 2.200 19.47 1 1 4 1 #> Toyota Corolla 33.9 4 71.1 65 4.22 1.835 19.90 1 1 4 1 mtcars %>% poorman::arrange(poorman::across(poorman::all_of(arrange_vars))) %>% head() #> Error in switch(type, `:` = select_seq(x), `!` = select_negate(x), `-` = select_minus(x), : EXPR must be a length 1 vector ``` Created on 2021-01-05 by the [reprex package](https://reprex.tidyverse.org) (v0.3.0)

Similar, but different when grouped.

Grouped ``` r `%>%` <- poorman::`%>%` arrange_vars <- c("carb", "am") # JUST ARRANGE mtcars %>% dplyr::group_by(cyl) %>% dplyr::arrange(arrange_vars) %>% head() #> Error: arrange() failed at implicit mutate() step. #> ✖ Could not create a temporary column for `..1`. #> ℹ `..1` is `arrange_vars`. mtcars %>% poorman::group_by(cyl) %>% poorman::arrange(arrange_vars) %>% head() #> mpg cyl disp hp drat wt qsec vs am gear carb #> Merc 240D 24.4 4 146.7 62 3.69 3.190 20.00 1 0 4 2 #> Datsun 710 22.8 4 108.0 93 3.85 2.320 18.61 1 1 4 1 #> Mazda RX4 Wag 21.0 6 160.0 110 3.90 2.875 17.02 0 1 4 4 #> Mazda RX4 21.0 6 160.0 110 3.90 2.620 16.46 0 1 4 4 #> Duster 360 14.3 8 360.0 245 3.21 3.570 15.84 0 0 3 4 #> Hornet Sportabout 18.7 8 360.0 175 3.15 3.440 17.02 0 0 3 2 # ARRANGE USING ACROSS (WORKS BUT MESSAGES) mtcars %>% dplyr::group_by(cyl) %>% dplyr::arrange(dplyr::across(arrange_vars)) %>% head() #> Note: Using an external vector in selections is ambiguous. #> ℹ Use `all_of(arrange_vars)` instead of `arrange_vars` to silence this message. #> ℹ See . #> This message is displayed once per session. #> # A tibble: 6 x 11 #> # Groups: cyl [2] #> mpg cyl disp hp drat wt qsec vs am gear carb #> #> 1 21.4 6 258 110 3.08 3.22 19.4 1 0 3 1 #> 2 18.1 6 225 105 2.76 3.46 20.2 1 0 3 1 #> 3 21.5 4 120. 97 3.7 2.46 20.0 1 0 3 1 #> 4 22.8 4 108 93 3.85 2.32 18.6 1 1 4 1 #> 5 32.4 4 78.7 66 4.08 2.2 19.5 1 1 4 1 #> 6 33.9 4 71.1 65 4.22 1.84 19.9 1 1 4 1 mtcars %>% poorman::group_by(cyl) %>% poorman::arrange(poorman::across((arrange_vars))) %>% head() #> mpg cyl disp hp drat wt qsec vs am gear carb #> NA NA NA NA NA NA NA NA NA NA NA NA #> NA.1 NA NA NA NA NA NA NA NA NA NA NA #> NA.2 NA NA NA NA NA NA NA NA NA NA NA #> Datsun 710 22.8 4 108.0 93 3.85 2.320 18.61 1 1 4 1 #> Fiat 128 32.4 4 78.7 66 4.08 2.200 19.47 1 1 4 1 #> Toyota Corolla 33.9 4 71.1 65 4.22 1.835 19.90 1 1 4 1 # ARRANGE "CORRECTLY" USING ACROSS WITH TIDYSELECT-ER mtcars %>% dplyr::group_by(cyl) %>% dplyr::arrange(dplyr::across(tidyselect::all_of(arrange_vars))) %>% head() #> # A tibble: 6 x 11 #> # Groups: cyl [2] #> mpg cyl disp hp drat wt qsec vs am gear carb #> #> 1 21.4 6 258 110 3.08 3.22 19.4 1 0 3 1 #> 2 18.1 6 225 105 2.76 3.46 20.2 1 0 3 1 #> 3 21.5 4 120. 97 3.7 2.46 20.0 1 0 3 1 #> 4 22.8 4 108 93 3.85 2.32 18.6 1 1 4 1 #> 5 32.4 4 78.7 66 4.08 2.2 19.5 1 1 4 1 #> 6 33.9 4 71.1 65 4.22 1.84 19.9 1 1 4 1 mtcars %>% poorman::group_by(cyl) %>% poorman::arrange(poorman::across(poorman::all_of(arrange_vars))) %>% head() #> Error in switch(type, `:` = select_seq(x), `!` = select_negate(x), `-` = select_minus(x), : EXPR must be a length 1 vector ``` Created on 2021-01-05 by the [reprex package](https://reprex.tidyverse.org) (v0.3.0)

Expected behavior The output of each paired dplyr + poorman scenario would match

System Information: Please detail the following information

R.version.string
#> [1] "R version 4.0.3 (2020-10-10)"
packageVersion("poorman")
#> [1] '0.2.4.4'
Additional Details Per the [`dplyr::arrange` docs](https://dplyr.tidyverse.org/reference/arrange.html), `arrange` should ignore grouping variables unless `.by_group` is set to `TRUE` I believe the "proper" tidy way to do this is _technically_: ``` r library(rlang) `%>%` <- magrittr::`%>%` arrange_vars <- c("carb", "am") sym_arrange_vars <- rlang::syms(arrange_vars) # !!! = rlang::`!!!` mtcars %>% dplyr::arrange(!!! sym_arrange_vars) #> mpg cyl disp hp drat wt qsec vs am gear carb #> Hornet 4 Drive 21.4 6 258.0 110 3.08 3.215 19.44 1 0 3 1 #> Valiant 18.1 6 225.0 105 2.76 3.460 20.22 1 0 3 1 #> Toyota Corona 21.5 4 120.1 97 3.70 2.465 20.01 1 0 3 1 #> Datsun 710 22.8 4 108.0 93 3.85 2.320 18.61 1 1 4 1 #> Fiat 128 32.4 4 78.7 66 4.08 2.200 19.47 1 1 4 1 #> Toyota Corolla 33.9 4 71.1 65 4.22 1.835 19.90 1 1 4 1 #> Fiat X1-9 27.3 4 79.0 66 4.08 1.935 18.90 1 1 4 1 #> Hornet Sportabout 18.7 8 360.0 175 3.15 3.440 17.02 0 0 3 2 #> Merc 240D 24.4 4 146.7 62 3.69 3.190 20.00 1 0 4 2 #> Merc 230 22.8 4 140.8 95 3.92 3.150 22.90 1 0 4 2 #> Dodge Challenger 15.5 8 318.0 150 2.76 3.520 16.87 0 0 3 2 #> AMC Javelin 15.2 8 304.0 150 3.15 3.435 17.30 0 0 3 2 #> Pontiac Firebird 19.2 8 400.0 175 3.08 3.845 17.05 0 0 3 2 #> Honda Civic 30.4 4 75.7 52 4.93 1.615 18.52 1 1 4 2 #> Porsche 914-2 26.0 4 120.3 91 4.43 2.140 16.70 0 1 5 2 #> Lotus Europa 30.4 4 95.1 113 3.77 1.513 16.90 1 1 5 2 #> Volvo 142E 21.4 4 121.0 109 4.11 2.780 18.60 1 1 4 2 ... ```
nathaneastwood commented 3 years ago

This is a good find, thanks. I'm going to have a think on this one and will probably rewrite the arrange() function. Currently there are a couple of issues with it, for example

r$> arrange(mtcars, 2)                                                                                                                                           
          mpg cyl disp  hp drat   wt  qsec vs am gear carb
Mazda RX4  21   6  160 110  3.9 2.62 16.46  0  1    4    4

The current implementation orders the number 2, returning 1, and so returns the first row. It's not ideal. To be fair, the implementation of arrange() was written 8 months ago when I just wanted a "working version". So it hasn't had a lot of attention.

alex-gable commented 3 years ago

I'd probably need some pointers, but let me know if you'd want some help on this

nathaneastwood commented 3 years ago

TL;DR poorman::arrange() needs a complete rewrite.

Thanks @alex-gable. My first task is to figure out how dplyr::arrange() works, i.e. what inputs does it accept. It would appear that the following work:

I also need to decide how to determine the order based on whether the user has given descending information, e.g. -mpg or poorman::desc(mpg).

Next I have to figure out how to implement all of this in base. Typically we arrange rows with the order() function, e.g.

rows <- order(mtcars[, "cyl"], mtcars[, "am"]) 
mtcars[rows, ]

There are of course other ways to do it.

Since dplyr::arrange() allows the user to perform mutations, I think this will mean I need to transmute() or mutate() and then order the resulting data.frame. But I also need to ignore other inputs, take for example dplyr::arrange(mtcars, 2) - what is this referring to? A column named 2 or the second column? This, I suspect, is why you cannot directly use tidy-select semantics in dplyr::arrange() unless you use across() (or similar).