- Notifications
You must be signed in to change notification settings - Fork82
Closed
Description
Issue
Thebuiltin_handler_progressr() handler appears to signal too large progress amounts to theprogressr 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 (iin 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 beforeclean() completes.
Troubleshooting
builtin_handler_progressr() passes relative progress using theamount argument of theprogressr progressor;
Lines 163 to 169 inbc50350
| 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) | |
| } | |
| }, |
I suspect the calculation ofamount is incorrect (e.g. should it bebar$progressr_last <- bar$current?). Regardless,progressr (>= 0.8.0) [since 2021-06-09] supports specifying the absolute progress using argumentstep, so no need to calculate the relative amount of progress.
Patch
diff --git a/R/progress-server.R b/R/progress-server.Rindex 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 expectedprogressr 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()Rversion4.2.2 (2022-10-31)Platform:x86_64-pc-linux-gnu (64-bit)Runningunder:Ubuntu20.04.5LTSMatrixproducts:defaultBLAS:/home/hb/shared/software/CBI/R-4.2.2-gcc9/lib/R/lib/libRblas.soLAPACK:/home/hb/shared/software/CBI/R-4.2.2-gcc9/lib/R/lib/libRlapack.solocale: [1]LC_CTYPE=en_US.UTF-8LC_NUMERIC=C [3]LC_TIME=en_US.UTF-8LC_COLLATE=en_US.UTF-8 [5]LC_MONETARY=en_US.UTF-8LC_MESSAGES=en_US.UTF-8 [7]LC_PAPER=en_US.UTF-8LC_NAME=C [9]LC_ADDRESS=CLC_TELEPHONE=C [11]LC_MEASUREMENT=en_US.UTF-8LC_IDENTIFICATION=Cattachedbasepackages:[1]statsgraphicsgrDevicesutilsdatasetsmethodsbaseotherattachedpackages:[1]cli_3.5.0loadedviaa namespace (andnotattached):[1]compiler_4.2.2progressr_0.12.0digest_0.6.31
Metadata
Metadata
Assignees
Labels
No labels