Skip to content

Conversation

@olivroy
Copy link
Collaborator

@olivroy olivroy commented Jun 26, 2024

Avoid superseded mutate_all() replace this with is.na.data.frame(). (6x speed-up, but uses more memory)

Remove pipes in tests

Refactor some tests for clarity

Bench marks

aa <- naniar::oceanbuoys
library(dplyr)
#> 
#> Attachement du package : 'dplyr'
#> Les objets suivants sont masqués depuis 'package:stats':
#> 
#>     filter, lag
#> Les objets suivants sont masqués depuis 'package:base':
#> 
#>     intersect, setdiff, setequal, union
aa[1, 2] <- NaN
f <- function(x) {
  x[is.na(x)] <- NA
x
}

aa %>% dplyr::mutate_all( .funs = function(x) {
  x[is.nan(x)] <- NA 
  x
}) %>% waldo::compare(f(aa))
#> ✔ No differences
bench::mark(
  f(aa),
  aa |> dplyr::mutate(dplyr::across(dplyr::everything(), function(x) {
    x[is.nan(x)] <- NA 
    x
  })) ,
  aa %>% dplyr::mutate_all( .funs = function(x) {
    x[is.nan(x)] <- NA 
    x
  })
)
#> # A tibble: 3 × 6
#>   expression                           min   median `itr/sec` mem_alloc `gc/sec`
#>   <bch:expr>                      <bch:tm> <bch:tm>     <dbl> <bch:byt>    <dbl>
#> 1 f(aa)                            485.7µs  656.3µs     1334.    4.23MB     8.65
#> 2 dplyr::mutate(aa, dplyr::acros…   2.56ms    3.2ms      279.  910.29KB    11.3 
#> 3 aa %>% dplyr::mutate_all(.funs…   2.72ms   3.09ms      302.   56.16KB    10.7

Created on 2024-06-25 with reprex v2.1.0

note that base R doesn’t provide is.nan.data.frame() hence why we need to use is.na

@rich-iannone rich-iannone self-requested a review June 26, 2024 04:11
Copy link
Member

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you!

@rich-iannone rich-iannone merged commit a46ff48 into rstudio:master Jun 26, 2024
@olivroy olivroy deleted the summary-rows branch June 26, 2024 11:38
olivroy added a commit to olivroy/gt that referenced this pull request Jul 5, 2024
… `.by` instead of `group_by()`, `vctrs::vec_slice()`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants