knitr icon indicating copy to clipboard operation
knitr copied to clipboard

use a single-line progress bar, addresses #1880

Open mariusbarth opened this issue 4 years ago • 12 comments
trafficstars

Hi all,

along the lines of #1880, this is an attempt at creating a more compact progress bar.

  • creates a globally accessible, bespoke progress bar knit_progress()
  • create set_knit_progress() to update value and/or printed text
  • also use cat() (instead of message() for printing information about processed files, to prevent mixing of streams from stdout and stderr

If knitr::opts_knit$set(verbose = TRUE), new lines are added between the following lines:

processing file:  test.rmd
 chunk: setup                              |..............                  |  43%
 text                                      |..................              |  57%
 chunk: cars                               |..................              |  57%
 summary(cars)                             |..................              |  57%
 x <- sample(2e6)                          |.......................         |  71%
 text                                      |...........................     |  86%
 chunk: pressure                           |...........................     |  86%
 plot(pressure)                            |...........................     |  86%
 x <- sample(2e6)                          |................................| 100%
 text                                      |................................| 100%
output file:  test.knit.md 

If verbose = FALSE, code bits are not printed, and all lines (except proceccing file: test.rmd and output file: test.knit.md) are overwritten. I hope this is quiet close to what @hadley had in mind.

To avoid messages messing up the console (and the progress bar), ~~I now use capture.output(..., type = "message") to first capture messages and then cat() them above the progress bar. I'm fully aware that this might be a bit too much, but I couldn't come up with a better solution.~~ EDIT: I came up with a better solution, a proper message handler, and could switch back to withCallingHandlers().

If there are any necessary changes, just let me know.

mariusbarth avatar Aug 12 '21 12:08 mariusbarth

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 12 '21 12:08 CLAassistant

I just added some more changes that are intended to better manage the different output streams, especially messages that are printed to the console while knitting.

If my .Rmd contains something like this:

message("A message that will maybe hint me to a problem.")
message("Another message from the same code chunk.")

it will now be printed in the console as such: Bildschirmfoto von 2021-08-12 23-45-13

If verbose = TRUE, it looks like the following: Bildschirmfoto von 2021-08-12 23-46-24


With the current code, it would be super easy to get similar output for warnings, which I think would be very helpful when debugging. However, this would only work if we set options(warn = 1) in the respective R session.

mariusbarth avatar Aug 12 '21 21:08 mariusbarth

Hello, I just stumbled on this PR, and I was wondering if you had thought of a way to include chunk options as well. I find it useful to show them in the current progress bar, e.g:

 |...............................                                       |  44%
label: reg-all (with options) 
List of 2
 $ cache  : logi TRUE
 $ results: chr "asis"

  |...................................                                   |  50%
  ordinary text without R code

  |.......................................  

Otherwise, this looks nice to me, so thanks

etiennebacher avatar Sep 03 '21 10:09 etiennebacher

Hi @etiennebacher, it would definitely be possible to print chunk options, though it would require some more changes. The challenge is that what can be overwritten (i.e., "updated") is only the current line, so everything that is supposed to be updated has to fit into a single line of the console. Printing chunk options to this single line would most likely make it rather unwieldy, which is why I opted against adding them.

However, while working on the progress bar, I figured that the current implementation serves two rather distinct and partially conflicting purposes: The first being to inform about the processing status of the document, and the second to provide some kind of log. The new progress bar as I implemented it here (with verbose = FALSE) clearly prioritizes the "processing status" aspect (e.g., information is overwritten, everything fits into a single line). By contrast, the same does not apply to the verbose = TRUE option, and I think this might be the best place to print information about chunk options. If you (and maybe @yihui) agree, I would be more than happy to provide a draft for printing chunk options to the verbose = TRUE output.

mariusbarth avatar Sep 03 '21 14:09 mariusbarth

Well I don't have a clear-cut opinion on how chunk options info should be displayed and implemented, I just wanted to make sure that they wouldn't be forgotten in the process of updating progress indicators. I see that you have already thought about that and have some ideas on how to include them, so I'll step aside and let you and knitr maintainers discuss about that ;)

etiennebacher avatar Sep 03 '21 15:09 etiennebacher

@etiennebacher Thanks! I'll review this PR a couple of weeks later. For now, I just wonder if we could avoid copying code from base R.

yihui avatar Sep 04 '21 16:09 yihui

For the sake of at least mentioning them before committing to a decision, there are a few CRAN packages which offer progress bar tools:

  • progress (https://github.com/r-lib/progress) which has a few dependencies though
  • progressr (https://github.com/HenrikBengtsson/progressr) which has fewer deps and a minimal API with strict rule

There are probably other, but it interesting to know what is out there. Specifically because they have some options for the user to tweak the style and control the progress bar, while the developer (us) still controls what is reported and when.

Regarding the discussion above, I agree we should try to not copying directly from base R. I believe we could adapt the logic to our need, and implement using usual style of this package.

Here what I think we need and could do for the progress bar:

  • Use an environment to store state of the progress
  • Some functions would allow to create, init, update and clear the progress bar.
    • create would return the env with some default structure: pb <- knit_progress_create(...)
    • init would initiate the environment to set the init state of the progress: pb <- knit_progress_init(pb, ...)
    • update would modify the env values during the progression: pb <- knit_progress_update(pb, ...)
    • clear would would be called when progress is done to close the progress bar : knit_progress_clear(pb)
  • Another one would allow to print the state of the progress bar: knit_progress_show(pb, ...)

I guess the logic is similar than txtProgressBar() and other progress bar packages.

Anyway, just sharing thoughts on this. @yihui, feel free to reach out if you want to brainstorm more on this.

Small feedback on the current PR: That would be best if we can start avoiding using some stringr functions. That would help us later for #1549

cderv avatar Sep 06 '21 13:09 cderv

Hi @cderv and @yihui,

I just removed the stringr dependency. Regarding the copied code from base R: It was my impression that neither base-R functions, nor any of the simple progress-bar packages, provides progress bars where a message is updated side-by-side with a status bar. The progress package could be used to accomplish this goal, but I figured with its dependency graph, it was a bit over to the top. Therefore, I chose to use modified base-R code, because this seemed to me the most solid foundation to start from.

mariusbarth avatar Sep 13 '21 15:09 mariusbarth

@mariusbarth:

However, while working on the progress bar, I figured that the current implementation serves two rather distinct and partially conflicting purposes: The first being to inform about the processing status of the document, and the second to provide some kind of log.

⬆️ this.

Perhaps it might make sense separate these two concerns:

  1. let knitr::knit(quiet = FALSE) control the emission of a log on stdout, including options, whatever else people find useful. (This would have always have to be knitr specific) This output can, even with knitr::opts_knit$set(verbose = FALSE) be more verbose, than people might want a progress bar to be.
  2. let a new knitr::knit(progress = FALSE) control the emission of a progressr::progressor() object, whose handling/presentation is determined by the caller (that's @HenrikBengtsson design). Via progress, this could default to the kind of condensed progress info specced in #1880, perhaps simply set as a default option with something like:
    knitr::opts_knit$set(progress_handler = progressr::handlers(progressr::handler_progress())
    
    (I've separately logged implementing 2) in https://github.com/yihui/knitr/issues/2077)

This may conflicts/overlap with existing verbose and progress knitr options, though if I understand the progressr docs correctly, a progressr progress bar need not interfere with other stdout, but is simply presented underneath it.

maxheld83 avatar Dec 02 '21 12:12 maxheld83

@cderv 👍

Here what I think we need and could do for the progress bar:

  • Use an environment to store state of the progress

  • Some functions would allow to create, init, update and clear the progress bar.

    • create would return the env with some default structure: pb <- knit_progress_create(...)
    • init would initiate the environment to set the init state of the progress: pb <- knit_progress_init(pb, ...)
    • update would modify the env values during the progression: pb <- knit_progress_update(pb, ...)
    • clear would would be called when progress is done to close the progress bar : knit_progress_clear(pb)
  • Another one would allow to print the state of the progress bar: knit_progress_show(pb, ...)

Isn't this what progressr does (already)? I think it'd be great to reuse that.

maxheld83 avatar Dec 02 '21 12:12 maxheld83

I want to invest some time to contribute in this PR. I understand from the discussion that packages for progress bar are available (like progress and progressr), but, they bring unwanted dependencies. Hence, a custom solution using minimal dependencies is more desirable. Because of it, I will use @mariusbarth work as a start;

I very much agree with @maxheld83 that the progress bar is being used for two distinct purposes (report progress, and, logging information), and, that these systems should be separated. I can try to build a logging system in the way, but, maybe it should be treated in a different PR, I guess....

To maintain the code standard of knitr, I translated some <- of @mariusbarth to equal signs =;

I like very much of @cderv suggestions to store the current state of the progress bar in a environment. And to reuse the code standard of knitr, I thought as a good idea, to use new_defaults() to build such environment. In other words, knit_progress_create() will create a environment using new_defaults(), and return such environment as a result;

Therefore, knit_progress_update() will use the $set() method to update the values in this environment, and knit_progress_show() will use the $get() method to use the variables in this environment to build and print the progress bar in the console;

For now, these ideas make sense to you? Did I understand something wrong? Is what you all meant in this discussion?

pedropark99 avatar Sep 16 '22 11:09 pedropark99

@pedropark99 thanks for your interest. We are looking closer to the topic of progress bar for our next version. Main discussion is in #1880

We will take some time to think about the new design based on all the feedbacks for the different issues. Please do share your views so that we take it into account. Especially, we need to take time develop clearly the goals to identify the best path to tackle them before doing anything new in the code.

cderv avatar Sep 16 '22 13:09 cderv

Just a quick update: the current dev version of knitr uses a single-line progress bar now, and the PR #2196 allows users to choose their own custom progress bars. Feedback welcome!

yihui avatar Nov 22 '22 16:11 yihui

I've just verified that PR#2196 works with progressr. See my https://github.com/yihui/knitr/pull/2196#issuecomment-1324209523 for how to set it up.

Regarding @mariusbarth's comment on:

If knitr::opts_knit$set(verbose = TRUE), new lines are added between the following lines: ...

Note with progressr, this is not a problem, because progressr buffers any stdout, messages, and warnings until the next progress update is rendered. This means you can use cat(), message(), ... just as when there wasn't any progress output to worry about (https://progressr.futureverse.org/#use-regular-output-as-usual-alongside-progress-updates).

HenrikBengtsson avatar Nov 22 '22 20:11 HenrikBengtsson