feat: Math operations on num() objects now pass additional arguments to the mathematical function

[gvelasq]

Fixes #659

Thank you for considering a pull request. As I looked into this, I noticed that sum() and other functions that take additional arguments (e.g., mean(), prod()) are handled by vec_math_base() instead of vec_arith_base(). I passed the ellipsis to vec_math_base() which now performs the correct calculation, and I added a test which passes.

# current behavior
library(pillar)
sum(num(c(1:3, NA)), na.rm = TRUE)
#> <pillar_num[1]>
#> [1] NA

# proposed behavior
devtools::load_all("~/rrr/pillar", quiet = TRUE)
sum(num(c(1:3, NA)), na.rm = TRUE)
#> <pillar_num[1]>
#> [1] 6

^{Created on 2024-06-16 with reprex v2.1.0}

[krlmlr]

Thanks. Why do the tests fail?

[krlmlr]

Do other methods also need to forward the dots?

[gvelasq]

Thank you for reviewing. As far as I can tell, I don't believe other methods need to forward the dots. vec_math_base() only takes function names as a character vector.

library(sloop)
library(vctrs)
library(pillar)

vec_math_base("sum", c(1:3, NA), na.rm = TRUE)
#> [1] 6

s3_dispatch(vec_math_base("sum", c(1:3, NA), na.rm = TRUE))
#>    vec_math_base.character
#>    vec_math_base.default

s3_methods_class("pillar_num")
#> # A tibble: 9 × 4
#>   generic            class      visible source             
#>   <chr>              <chr>      <lgl>   <chr>              
#> 1 format             pillar_num FALSE   registered S3method
#> 2 vec_arith.numeric  pillar_num FALSE   registered S3method
#> 3 vec_arith          pillar_num FALSE   registered S3method
#> 4 vec_cast.double    pillar_num FALSE   registered S3method
#> 5 vec_math           pillar_num FALSE   registered S3method
#> 6 vec_ptype_abbr     pillar_num FALSE   registered S3method
#> 7 vec_ptype_full     pillar_num FALSE   registered S3method
#> 8 vec_ptype2.double  pillar_num FALSE   registered S3method
#> 9 vec_ptype2.integer pillar_num FALSE   registered S3method

^{Created on 2024-06-20 with reprex v2.1.0}

[gvelasq]

I reverted NEWS.md because now I see that this file is managed by fledge.

[gvelasq]

Thanks. Why do the tests fail?

I believe that the tests failed because passing the dots to vec_math_base() fixed a bug in this test snapshot. It's easier to reason about the log transformation using the test dataset's y variable on the y-axis, rather than the z variable on the y-axis as in the snapshot, so I am showing both examples in the reprex below. I compared the current and proposed behavior of pillar::scale_y_num(transform = "log10") to ggplot2::scale_y_continuous(transform = "log10").

One minor point is that the trans argument to ggplot2::continuous_scale() has been deprecated in favor of transform. I fixed this in the most recent commit to the pull request.

library(ggplot2)
library(pillar)

# load current scale_y_num()
library(tibble)

# data with num() objects used in test/testthat/test-ggplot2.R
df1 <-
  tibble(
    x = num((1:10) / 100, fixed_exponent = -3, notation = "eng"),
    y = num((1:10) / 100, scale = 100, label = "%"),
    z = num(10^(-5:4), notation = "si")
  )

# plain data
df2 <-
  tibble(
    x = seq(10, 100, by = 10),
    y = 1:10,
    z = 10^(-5:4)
  )

# current scale_y_num() on num() objects, x-y
p1xy <-
  ggplot(df1, aes(x, y)) +
  geom_point() +
  scale_y_num(transform = "log10") +
  theme_minimal(36)
p1xy
#> Warning: The `scale_name` argument of `continuous_scale()` is deprecated as of ggplot2
#> 3.5.0.
#> This warning is displayed once every 8 hours.
#> Call `lifecycle::last_lifecycle_warnings()` to see where this warning was
#> generated.

# current scale_y_num() on num() objects, x-z
p1xz <-
  ggplot(df1, aes(x, z)) +
  geom_point() +
  scale_y_num(transform = "log10") +
  theme_minimal(36)
p1xz

# current scale_y_continuous() on plain data, x-y
p2xy <-
  ggplot(df2, aes(x, y)) +
  geom_point() +
  scale_y_continuous(transform = "log10") +
  theme_minimal(36)
p2xy

# current scale_y_continuous() on plain data, x-z
p2xz <-
  ggplot(df2, aes(x, z)) +
  geom_point() +
  scale_y_continuous(transform = "log10") +
  theme_minimal(36)
p2xz

# load proposed scale_y_num()
devtools::load_all("~/rrr/pillar")
#> ℹ Loading pillar

# proposed scale_y_num() on num() objects, x-y
p3xy <-
  ggplot(df1, aes(x, y)) +
  geom_point() +
  scale_y_num(transform = "log10") +
  theme_minimal(36)
p3xy

# proposed scale_y_num() on num() objects, x-z
p3xz <-
  ggplot(df1, aes(x, z)) +
  geom_point() +
  scale_y_num(transform = "log10") +
  theme_minimal(36)
p3xz

^{Created on 2024-06-21 with reprex v2.1.0}

[krlmlr]

Thanks!