Skip to content
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

Refactor tests and summary_rows() #1732

Merged
merged 6 commits into from
Jun 26, 2024
Merged

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
11 checks passed
@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