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

Two clarifications on the progress bar implementation #1047

Closed
DavisVaughan opened this issue Jan 17, 2023 · 3 comments
Closed

Two clarifications on the progress bar implementation #1047

DavisVaughan opened this issue Jan 17, 2023 · 3 comments

Comments

@DavisVaughan
Copy link
Member

In particular, this example has two quirks:

library(purrr)

x <- 1:5

map(x, .progress = TRUE, function(elt) {
  Sys.sleep(4)
})
  • The progress bar doesn't show up at all until after the first iteration (i.e. about 4 seconds in). I don't know if this is a consequence of show_after or the fact that we are setting i = 0 on the first iteration.
  • The progress bar never actually hits 100%. On the last iteration we set i = 4, which corresponds to 80% which does show up, but then we do the last iteration and exit the loop. I assume that calling cli_progress_done() will mark it as 100%, and that does seem to be the case if you set clear = FALSE, but I still wonder if we should be using i + 1 in the loop.

CC @gaborcsardi

@hadley
Copy link
Member

hadley commented Jan 17, 2023

That reasoning makes sense to me.

@gaborcsardi
Copy link
Member

gaborcsardi commented Jan 17, 2023

I'd like to check and make sure that it was purposeful that cli_progress_set() uses i rather than i + 1 here in the loop

I'd also like to see if we should consider putting cli_progress_set() after the call to .f (brought about by thinking about DavisVaughan/furrr#248)

Yes, it is on purpose. We set it to i because we haven't done the ith step yet. I.e. we set it to i+1 after the ith step, just like you want.

We set it before the call to .f, so if the developer or the user sets show_after = 0, then it is shown before a potentially lengthy first iteration.

The progress bar doesn't show up at all until after the first iteration (i.e. about 4 seconds in). I don't know if this is a consequence of show_after or the fact that we are setting i = 0 on the first iteration.

Yes, it is because of show_after.

The progress bar never actually hits 100%. On the last iteration we set i = 4, which corresponds to 80% which does show up, but then we do the last iteration and exit the loop. I assume that calling cli_progress_done() will mark it as 100%, and that does seem to be the case if you set clear = FALSE, but I still wonder if we should be using i + 1 in the loop.

Yes, cli_progress_done() will mark it as 100%, and another cli_progress_set() call would be redundant.

@DavisVaughan
Copy link
Member Author

Okay, that all makes sense to me. I agree that it is fine as is.

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

No branches or pull requests

3 participants