builtin_handler_progressr(): Incorrect amount of progress (with PATCH) #558
Description
Issue
The builtin_handler_progressr() handler appears to signal too large progress amounts to the progressr progressor function. For example,
library(cli)
options(cli.progress_show_after = 0.0)
options(cli.progress_handlers = "progressr")
progressr::handlers(global = TRUE)
progressr::handlers("debug")
clean <- function(n = 10L) {
cli_progress_bar("Cleaning data", total = n)
for (i in seq_len(n)) {
Sys.sleep(5/n)
cli_progress_update()
}
cli_progress_done()
}
clean()outputs:
[10:11:20.203]
[10:11:20.723] (0.000s => +0.007s) initiate: 0/10 (+0) '' {clear=TRUE, enabled=TRUE, status=}
[10:11:20.799] (0.075s => +0.075s) update: 1/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:11:21.301] (0.578s => +0.001s) update: 3/10 (+2) '' {clear=TRUE, enabled=TRUE, status=}
[10:11:21.804] (1.080s => +0.001s) update: 6/10 (+3) '' {clear=TRUE, enabled=TRUE, status=}
[10:11:22.306] (1.583s => +0.001s) update: 10/10 (+4) '' {clear=TRUE, enabled=TRUE, status=}
[10:11:25.316] (4.593s => +0.000s) shutdown: 10/10 (+0) '' {clear=TRUE, enabled=TRUE, status=}
[10:11:25.333]
Note how it (i) makes bigger and bigger steps (they should all be +1), and (ii) reaches 100% way before clean() completes.
Troubleshooting
builtin_handler_progressr() passes relative progress using the amount argument of the progressr progressor;
Lines 163 to 169 in bc50350
I suspect the calculation of amount is incorrect (e.g. should it be bar$progressr_last <- bar$current?). Regardless, progressr (>= 0.8.0) [since 2021-06-09] supports specifying the absolute progress using argument step, so no need to calculate the relative amount of progress.
Patch
diff --git a/R/progress-server.R b/R/progress-server.R
index e1a6c61d..6afed864 100644
--- a/R/progress-server.R
+++ b/R/progress-server.R
@@ -150,7 +150,6 @@ builtin_handler_cli <- list(
builtin_handler_progressr <- list(
add = function(bar, .envir) {
steps <- if (is.na(bar$total)) 0 else bar$total
- bar$progressr_last <- 0L
bar$progressr_progressor <- asNamespace("progressr")$progressor(
steps,
auto_finish = FALSE,
@@ -161,18 +160,16 @@ builtin_handler_progressr <- list(
},
set = function(bar, .envir) {
- amount <- bar$current - bar$progressr_last
bar$last <- bar$current
- if (!is.null(bar$progressr_progressor) && amount > 0) {
- bar$progressr_progressor(amount = amount)
+ if (!is.null(bar$progressr_progressor)) {
+ bar$progressr_progressor(step = bar$current)
}
},
complete = function(bar, .envir, result) {
- amount <- bar$current - bar$progressr_last
bar$last <- bar$current
- if (!is.null(bar$progressr_progressor) && amount > 0) {
- bar$progressr_progressor(amount = amount, type = "finish")
+ if (!is.null(bar$progressr_progressor)) {
+ bar$progressr_progressor(step = bar$current, type = "finish")
}
},Using the above patch produces the expected progressr progress signals;
[10:21:46.923]
[10:21:47.451] (0.000s => +0.004s) initiate: 0/10 (+0) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:47.498] (0.047s => +0.047s) update: 1/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:47.999] (0.549s => +0.000s) update: 2/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:48.501] (1.050s => +0.001s) update: 3/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:49.003] (1.552s => +0.000s) update: 4/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:49.505] (2.054s => +0.000s) update: 5/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:50.007] (2.556s => +0.000s) update: 6/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:50.509] (3.058s => +0.001s) update: 7/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:51.010] (3.559s => +0.000s) update: 8/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:51.512] (4.061s => +0.001s) update: 9/10 (+1) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:52.014] (4.563s => +0.000s) shutdown: 9/10 (+0) '' {clear=TRUE, enabled=TRUE, status=}
[10:21:52.022]Session info
> sessionInfo()
R version 4.2.2 (2022-10-31)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 20.04.5 LTS
Matrix products: default
BLAS: /home/hb/shared/software/CBI/R-4.2.2-gcc9/lib/R/lib/libRblas.so
LAPACK: /home/hb/shared/software/CBI/R-4.2.2-gcc9/lib/R/lib/libRlapack.so
locale:
[1] LC_CTYPE=en_US.UTF-8 LC_NUMERIC=C
[3] LC_TIME=en_US.UTF-8 LC_COLLATE=en_US.UTF-8
[5] LC_MONETARY=en_US.UTF-8 LC_MESSAGES=en_US.UTF-8
[7] LC_PAPER=en_US.UTF-8 LC_NAME=C
[9] LC_ADDRESS=C LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C
attached base packages:
[1] stats graphics grDevices utils datasets methods base
other attached packages:
[1] cli_3.5.0
loaded via a namespace (and not attached):
[1] compiler_4.2.2 progressr_0.12.0 digest_0.6.31