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

Persistent progress_bar after interrupting map loop in the middle #1024

Closed
brunomioto opened this issue Dec 21, 2022 · 9 comments · Fixed by #1026
Closed

Persistent progress_bar after interrupting map loop in the middle #1024

brunomioto opened this issue Dec 21, 2022 · 9 comments · Fixed by #1026

Comments

@brunomioto
Copy link

brunomioto commented Dec 21, 2022

I'm using it in a personal function, but will try to reproduce it asap.
I run the function, progress bars appears and everything works fine (it even disappears as expected).
But if I interrupt this map loop with Esc, the progress bar keeping appearing after every code I ran

@brunomioto
Copy link
Author

brunomioto commented Dec 21, 2022

library(purrr)
  
example_fun <- function(query){
  
    
    res <- purrr::map(query, .f = function(x) {
      Sys.sleep(1)
    }, .progress = list(format = "Query {cli::pb_current+1}/{cli::pb_total} | {cli::pb_bar} {cli::pb_percent} [{cli::pb_elapsed}]",
                        show_after = 0,
                        clear = TRUE)
    )%>%
      purrr::list_rbind()

  return(res)
  
}

#First example - Pressed Esc in 33%
example_fun(c("a","b","c"))

#Second example - Run normally
example_fun(c("a","b","c"))

image

@hadley
Copy link
Member

hadley commented Dec 21, 2022

I can't reproduce this so we'll need to get some more info. Can you please run the following code?

packageVersion("cli")
sessioninfo::platform_info()

@brunomioto
Copy link
Author

packageVersion("cli")
#> [1] '3.4.1'
sessioninfo::platform_info()
#>  setting  value
#>  version  R version 4.1.1 (2021-08-10)
#>  os       Windows 10 x64 (build 19044)
#>  system   x86_64, mingw32
#>  ui       RTerm
#>  language (EN)
#>  collate  Portuguese_Brazil.1252
#>  ctype    Portuguese_Brazil.1252
#>  tz       America/Sao_Paulo
#>  date     2022-12-21
#>  pandoc   2.19.2 @ C:/Program Files/RStudio/resources/app/bin/quarto/bin/tools/ (via rmarkdown)

Created on 2022-12-21 with reprex v2.0.2

@hadley
Copy link
Member

hadley commented Dec 21, 2022

I'd start by restarting R and installing the latest cli (3.5).

@DavisVaughan
Copy link
Member

DavisVaughan commented Dec 21, 2022

Screen Shot 2022-12-21 at 6 14 41 PM

I can reproduce this too actually. With CRAN cli.

As @brunomioto said, if you run the first call of example_fun() and hit Esc to stop the progress of the function before it finishes, then run it again, it seems like the progress bar from the first run is trying to be "restored" because it hasn't been closed properly

@DavisVaughan
Copy link
Member

I wonder if it is because this cli_progress_done() is never hit?

cli_progress_done(bar);

Because of this R_CheckUserInterrupt() call?

R_CheckUserInterrupt();

@DavisVaughan
Copy link
Member

DavisVaughan commented Dec 21, 2022

Another way to reproduce is to cause a stop() on, say, the 2nd index. And then call the function again but without the stop()

It is possible we need to use R_ExecWithCleanup() and close the progress bar in the cleanup step.

library(purrr)

example_fun <- function(query){
  
  
  res <- purrr::map(query, .f = function(x) {
    if (x == "b") {
      stop("oh no")
    }
  }, .progress = list(format = "Query {cli::pb_current+1}/{cli::pb_total} | {cli::pb_bar} {cli::pb_percent} [{cli::pb_elapsed}]",
                      show_after = 0,
                      clear = TRUE)
  )%>%
    purrr::list_rbind()
  
  return(res)
  
}

#First example - Pressed Esc in 33%
example_fun(c("a","b","c"))

#Second example - Run normally
example_fun(c("a","c"))

@hadley
Copy link
Member

hadley commented Dec 22, 2022

cc @gaborcsardi

@gaborcsardi
Copy link
Member

gaborcsardi commented Dec 22, 2022

Yeah, there are two ways to fix this afaict. First is by adding cleancall support to purrr, i.e. embedding cleancall.{h,c} into purrr, and using it instead of .Call. Then this will be called: https://github.com/r-lib/cli/blob/bc503509cddb65d0007aeb301936e27de76bc7d7/inst/include/cli/progress.h#L299
https://github.com/r-lib/cli/tree/bc503509cddb65d0007aeb301936e27de76bc7d7/tests/testthat/progresstest has an example.

Second is to implement an exit handler in purrr itself, without cleancall.

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 a pull request may close this issue.

4 participants