fmt_duration rounds incorrectly

[rcannood]

Prework

• Read and agree to the code of conduct and contributing guidelines.
• If there is already a relevant issue, whether open or closed, comment on the existing thread instead of posting a new issue.

Description

Sum of durations is incorrect.

Update: There is a smaller reproducible example in the next comment.
Update 2: PR #1375 contains a unit test for the described problem.
Update 3: PR #1375 now also contains a fix for the problem. However, the pre-existing unit tests are incorrect and still need to be updated.

Reproducible example

library(gt)

data.frame(duration = c(90L, 2160L)) %>%
  gt() %>%
  fmt_duration(
    columns = "duration",
    input_units = "minutes",
    output_units = c("hours", "minutes")
  ) %>%
  grand_summary_rows(
    columns = "duration",
    fns = list(Total ~ sum(.)),
    fmt = list(~fmt_duration(
        .,
        columns = "duration",
        input_units = "minutes",
        output_units = c("hours", "minutes")
    ))
  )

This results in the table:

	duration
	1h 30m
	36h
Total	37h 29m

Expected result

I would have expected the total to be 37h 30m:

	duration
	1h 30m
	36h
Total	37h 30m

Session info

R version 4.2.3 (2023-03-15)
Platform: x86_64-redhat-linux-gnu (64-bit)
Running under: Fedora Linux 37 (Workstation Edition)

Matrix products: default
BLAS/LAPACK: /usr/lib64/libflexiblas.so.3.3

locale:
 [1] LC_CTYPE=en_GB.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_GB.UTF-8        LC_COLLATE=en_GB.UTF-8    
 [5] LC_MONETARY=en_GB.UTF-8    LC_MESSAGES=en_GB.UTF-8   
 [7] LC_PAPER=en_GB.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_GB.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
[1] gt_0.9.0

loaded via a namespace (and not attached):
 [1] fansi_1.0.4      withr_2.5.0      digest_0.6.31    utf8_1.2.3      
 [5] dplyr_1.1.2      R6_2.5.1         lifecycle_1.0.3  jsonlite_1.8.7  
 [9] magrittr_2.0.3   pillar_1.9.0     rlang_1.1.1      cli_3.6.1       
[13] xml2_1.3.4       vctrs_0.6.3      generics_0.1.3   tools_4.2.3     
[17] glue_1.6.2       fastmap_1.1.1    compiler_4.2.3   pkgconfig_2.0.3 
[21] htmltools_0.5.5  tidyselect_1.2.0 sass_0.4.6       tibble_3.2.1 

[rcannood]

Update: This doesn't have anything to do with grand_summary_rows but only with fmt_duration, because the following also produces incorrect results.

MRE

library(gt)

data.frame(duration = seq(90L, 100L)) %>%
  gt() %>%
  fmt_duration(
    columns = "duration",
    input_units = "minutes",
    output_units = c("hours", "minutes")
  )

Result

duration
1h 30m
1h 31m
1h 31m
1h 33m
1h 34m
1h 35m
1h 36m
1h 37m
1h 37m
1h 39m
1h 40m

Expected result

duration
1h 30m
1h 31m
1h 32m
1h 33m
1h 34m
1h 35m
1h 36m
1h 37m
1h 38m
1h 39m
1h 40m

[rcannood]

I've added a unit test in #1375 :)

[rcannood]

After digging through the code a bit and figuring out how gt is organised, it seems this code is responsible for the error is this in values_to_durations():

  # Obtain the units of `x` if it is of the difftime class (and
  # drop difftime attrs with `as.numeric()`)
  if (inherits(x, "difftime")) {
    in_units <- units(x)
    x <- as.numeric(x)
  }

  if (inherits(x, "integer")) {
    x <- as.numeric(x)
  }

  # Should `in_units` be anything other than days then
  # convert all `x` values to days
  if (in_units != "days") {

    x <-
      switch(
        in_units,
        weeks = x * 7,
        hours = x / 24,
        mins = ,
        minutes = x / 1440,
        secs = ,
        seconds = x / 86400
      )
  }

I'm not sure why difftime and integers are being converted to numerics, but this definitely explains the behaviour that we're seeing.

[rcannood]

I added a fix for this issue in #1375. However, the unit tests are now failing, because the checks are incorrect.

For example, the first test checks:

  # Create an input tibble with a numeric column
  data_tbl_1 <-
    dplyr::tibble(
      num_1 = c(1.0030, 36323.005, 5.000003, -34.5, 31.6, 28.5, NA)
    )

  # Create a `gt_tbl` object with `gt()` and the
  # `data_tbl_1` dataset
  tab_1 <- gt(data_tbl_1)

  # Format the `num_1` column using the defaults for `fmt_duration()` and
  # ensuring the `input_units` are days
  expect_equal(
    (tab_1 %>%
       fmt_duration(columns = "num_1", input_units = "days") %>%
       render_formats_test(context = "html"))[["num_1"]],
    c(
      "1d 4m 19s", "5,189w 7m 11s", "5d", paste0("\U02212", "4w 6d 12h"),
      "4w 3d 14h 24m", "4w 12h", "NA"
    )
  )

However, 36323.005 days should be formatted as 5,189w 7m 12s and not 5,189w 7m 11s and so the unit test fails. This is because:

• 36323 days equals 5189w,
• .005 days equals 432s ( = .005 * 24 * 3600), or 7m 12s

[rcannood]

Hi @rich-iannone. I managed to solve my problem in #1375. Would you have some time to take a look at this?

[rich-iannone]

@rcannood Thank you very much for all of this! Will definitely review the PR.

[rich-iannone]

@rcannood #1375 is now merged. Thanks so much for fixing this long-standing problem!