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

Add labels to the default progress bar and allow users to provide a custom progress bar #2196

Merged
merged 5 commits into from
Nov 24, 2022

Conversation

yihui
Copy link
Owner

@yihui yihui commented Nov 19, 2022

Users can customize the progress bar via options(knitr.progress.fun = function(total, labels) {}). The function should create a progress bar and return two methods, update() and done(). txt_pb() is the default.

  • Document the knitr.progress.fun function (total is the total number of chunks; labels is the vector of chunk labels---for text chunks, the labels are empty strings).

An example of using the cli progress bar:

options(knitr.progress.fun = function(total, labels) {
  id = cli::cli_progress_bar(total = total, .auto_close = FALSE)
  list(
    update = function(i) {
      cli::cli_progress_update(id = id)
    },
    done = function() {
      cli::cli_process_done(id)
    }
  )
})

An example of progressr (not working---I'm not familiar with this package and couldn't figure out how to make it work):

options(knitr.progress.fun = function(total, labels) {
  p = progressr::progressor(total, auto_finish = FALSE, enable = TRUE)
  list(
    update = function(i) {
      p(message = sprintf('chunk: %s', labels[i]))
    },
    done = function() {
      p(type = 'finish')
    }
  )
})

To disable the progress bar:

options(knitr.progress.fun = FALSE)

Note that the global option knitr.progress.fun has to be set before knitr::knit() starts. That usually means this option needs to be set in .Rprofile.

@yihui yihui linked an issue Nov 22, 2022 that may be closed by this pull request
@yihui yihui mentioned this pull request Nov 22, 2022
@yihui yihui requested review from rich-iannone and cderv November 22, 2022 15:58
@yihui yihui linked an issue Nov 22, 2022 that may be closed by this pull request
3 tasks
Copy link
Collaborator

@rich-iannone rich-iannone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@hadley
Copy link
Contributor

hadley commented Nov 22, 2022

This seems like a nice improvement and it will make a lot of people happy. I think there are two small things that you might consider.

Firstly, I was a little surprised that labels was passed in the constructor, rather than being supply to the update function, e.g.

options(knitr.progress.fun = function(total) {
  id = cli::cli_progress_bar(total = total, .auto_close = FALSE)
  list(
    update = function(i, label = NULL) {
      cli::cli_progress_update(id = id)
    },
    done = function() {
      cli::cli_process_done(id)
    }
  )
})

And I think it would be nice if when the label was empty you automatically generated the standard "unnamed chunk {i}" label, to avoid every progress bar implementer having to do that themselves.


Secondly, you might also consider bundling two "default" progress bars with knitr, one that's used when stdout has been directed to a file (since txtProgressBar documents ugly output in this case). I think it could be something as simple as:

batch_pb = function(total, labels) {
   s = ifelse(labels == '', '', sprintf(' (%s)', labels))
   list(
     update = function(i) {
       cat(s[i], "\n")
     },
     done = function() {
     }
   )
 }

I don't know how you tell if stdout has been redirected, but cli presumably has some function that you could inline.

@HenrikBengtsson
Copy link
Contributor

HenrikBengtsson commented Nov 22, 2022

Excellent. For progressr, the following works:

options(knitr.progress.fun = function(total, labels) {
  ## When creating a progressor it registers an on.exit() that
  ## shuts it downw automatically when exiting the function.
  ## To prevent this, we need need to use on_exit = FALSE.
  p <- progressr::progressor(total, auto_finish = FALSE, on_exit = FALSE)
  
  list(
    update = function(i) {
      p(message = sprintf('chunk: %s', labels[i]))
    },
    done = function() {
      p(type = 'finish')
    }
  )
})

That'll signal progress conditions. For a user to "subscribe" to them, use:

progressr::handlers(global = TRUE)

To tweak the progress reporter, to say, progress, use:

progressr::handlers("progress")

A reporter for cli is still on the to-do list (futureverse/progressr#123)

I'll see if I can add something like

options(knitr.progress.fun = progressr::make_knitr_progress_fcn())

with options to configure the undo and the done steps (and possible also the "init" step).

@HenrikBengtsson
Copy link
Contributor

I don't know how you tell if stdout has been redirected ...

Maybe I misunderstand the comment, but shouldn't all progress output to the terminal just be sent to stderr, e.g. cat(file = stderr(), ...), pb <- utils::txtProgressBar(..., file = stderr()), ... There should be no R functions or packages capturing the stderr (if they do, I think it's a mistake, because it's not safe).

@hadley
Copy link
Contributor

hadley commented Nov 22, 2022

@HenrikBengtsson I mean outside of R; i.e. you're not rendering interactively but are running rmarkdown::render() or similar from a batch job with output redirected to a log file.

@yihui
Copy link
Owner Author

yihui commented Nov 22, 2022

Thanks @hadley and @HenrikBengtsson!

I was a little surprised that labels was passed in the constructor, rather than being supply to the update function

Passing to the constructor gives users a little more freedom, e.g., they can calculate the maximum width of all labels to pad enough spaces to shorter labels (this is what I did in the default progress function). The update() function can easily get the current label from labels[i]. In the cli example, I was unable to figure out how to print the label. I tried

cli::cli_progress_update(id = id, status = labels[i])

but it didn't show the labels. Perhaps I need to use type = "custom" and a custom format string with extra data? I don't know.

I think it would be nice if when the label was empty you automatically generated the standard "unnamed chunk {i}" label, to avoid every progress bar implementer having to do that themselves

They don't need to do that---labels already have unnamed-chunk-i for unnamed chunks. The label is empty only for text chunks.

one that's used when stdout has been directed to a file

I'll see if it's possible to check whether stdout has been redirected.

@hadley
Copy link
Contributor

hadley commented Nov 22, 2022

@yihui ok, that makes sense. What's a text chunk? The non-code contents of an rmd?

@yihui
Copy link
Owner Author

yihui commented Nov 23, 2022

Yes (prose/narratives).

@yihui
Copy link
Owner Author

yihui commented Nov 24, 2022

since txtProgressBar documents ugly output in this case

I'm not entirely sure what they meant by "ugly output". I tried the txtProgressBar() on a file connection, and I got multiple lines of progress bars, which is what I expected (\r won't work in this case so the single-line progress bar can't be redrawn on the same line).

It doesn't seem to be possible to tell if the stdout has been redirected. For cli, I think it writes the progress bar to stderr. If I try to capture it with sink(type = 'message'), I get multiple lines of progress bars in the output file (also as I expected).

For now, I'll just leave this issue behind until someone reports it and requests changes.

@yihui
Copy link
Owner Author

yihui commented Nov 24, 2022

BTW, options(knitr.progress.output = stderr()) can send the progress to stderr.

@yihui yihui merged commit f440712 into master Nov 24, 2022
@yihui yihui deleted the custom-progress-bar branch November 24, 2022 03:06
rich-iannone added a commit that referenced this pull request Dec 22, 2022
* master:
  make negative times 0
  replace highr:::spaces() with strrep() in base R
  support chunk options message/warning = NA
  close #2204: drop the support for cairoDevice
  amend 1a0f2cc: make line_count() a few times faster
  Replace several stringr-based function calls with base equivalents (#2202, #1549)
  see if this fix the ruby gems issue https://github.com/yihui/knitr/actions/runs/3682745993/jobs/6230638404
  stop importing xfun::isFALSE() and define isFALSE() only for R < 3.5.0
  Add labels to the default progress bar and allow users to provide a custom progress bar (#2196)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

emit progressr::progressor() progress bar from knit() Rethink the progress bar
4 participants