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

Bug: progress bars can only access global environment #1078

Open
DanChaltiel opened this issue May 2, 2023 · 4 comments
Open

Bug: progress bars can only access global environment #1078

DanChaltiel opened this issue May 2, 2023 · 4 comments
Labels
feature a feature request or enhancement map 🗺️ tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day

Comments

@DanChaltiel
Copy link

Hi,

Progress bars documentation says:

Format strings may contain R expressions to evaluate in braces.

However, while one would expect these expressions to evaluate in the current environment, it seems that they can only access variables declared in the global environment.

Here is a reprex.
As you can see, the progress bar doesn't know about y (parent environment) but knows about x (global environment). It doesn't know about z (current environment) either, you can test it.

library(purrr)
f = function(){
  z="z"
  pb = list(format="Computing {x} {y} {zz} {pb_bar} {pb_percent}")
  
  walk(1:5, ~{
    Sys.sleep(1)
  }, .progress=pb)
}

g=function(){
  y="y"
  f()
}

x="x"
g()
#> Error in `map()`:
#> i In index: 2.
#> Caused by error:
#> ! Could not evaluate cli `{}` expression: `y`.
#> Caused by error:
#> ! object 'y' not found
#> Backtrace:
#>      x
#>   1. +-global g()
#>   2. | \-global f()
#>   3. |   \-purrr::walk(...)
#>   4. |     \-purrr::map(.x, .f, ..., .progress = .progress)
#>   5. |       \-purrr:::map_("list", .x, .f, ..., .progress = .progress)
#>   6. |         +-purrr:::with_indexed_errors(...)
#>   7. |         | \-base::withCallingHandlers(...)
#>   8. |         \-purrr:::call_with_cleanup(...)
#>   9. +-cli:::progress_c_update(`<env>`)
#>  10. | \-h$add(pb, .envir = caller)
#>  11. |   \-cli::cli_status(...)
#>  12. |     +-cli:::cli__message(...)
#>  13. |     | \-"id" %in% names(args)
#>  14. |     \-cli:::glue_cmd(msg, .envir = .envir)
#>  15. |       \-cli:::glue(...)
#>  16. +-cli (local) `<fn>`("y")
#>  17. | +-.transformer(expr, .envir) %||% character()
#>  18. | \-cli (local) .transformer(expr, .envir)
#>  19. |   +-eval(expr, envir = envir) %??% ...
#>  20. |   | \-chain_error(expr, err, srcref = utils::getSrcref(sys.call()))
#>  21. |   |   \-base::withCallingHandlers(...)
#>  22. |   \-base::eval(expr, envir = envir)
#>  23. |     \-base::eval(expr, envir = envir)
#>  24. +-base::.handleSimpleError(...)
#>  25. | \-h(simpleError(msg, call))
#>  26. |   \-throw_error(err, parent = e)
#>  27. |     \-base::signalCondition(cond)
#>  28. \-purrr (local) `<fn>`(`<rlb__3_0>`)
#>  29.   \-cli::cli_abort(...)
#>  30.     \-rlang::abort(...)

Created on 2023-05-02 with reprex v2.0.2

@DanChaltiel
Copy link
Author

Actually, if it could also access .x or even its name, it would be tremendously useful 👍

@bairdj
Copy link

bairdj commented Jun 7, 2023

I also think being able to access .x would be a really helpful feature. For long running operations, it would be very useful to know which element is currently being processed. Sometimes I find that one element may take much longer than others, but it can be hard to determine which one it is (there are other ways to identify this of course, but this would be a quick and simple way)

@hadley hadley added feature a feature request or enhancement map 🗺️ labels Jul 26, 2023
@gaborcsardi
Copy link
Member

Actually, it is already possible, but maybe purrr could have a better default, and better docs would be nice:

f <- function() {
  b <- "bar"
  purrr::map(
    1:10, 
    function(x) Sys.sleep(1), 
    .progress = list(caller = environment(), format = "{b} {cli::pb_current}")
  )
}
invisible(f())

I.e. caller is the environment where the format string(s) are evaluated.

@hadley hadley added the tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day label Jul 15, 2024
@hadley
Copy link
Member

hadley commented Jul 15, 2024

I think this should be reasonably straightforward to implement — in map_, map2_, and pmap_, check isTRUE(.progress). If it is replace with the call suggested by @gaborcsardi using the .purrr_user_env as the environment. Then just needs a test and a news bullet. (Not sure any documentation changes are needed since I think this is the behaviour that folks would expect).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement map 🗺️ tidy-dev-day 🤓 Tidyverse Developer Day rstd.io/tidy-dev-day
Projects
None yet
Development

No branches or pull requests

4 participants