futureverse / futureverse/progressr
relay_progress() does not honor progress updates with amount = 0
- Dominant language
- R
- Stars
- 299
- Forks
- 11
- PR merge metrics
- No merged PRs in 30d
Description
The example is copied from [`716d2e:incl/progress_aggregator.R`](https://github.com/HenrikBengtsson/progressr/blob/716d2e750a7667303244f91736f0f8315577e25a/incl/progress_aggregator.R), but with the wrong amount of steps corrected from 4 to 8 (1: top-level `progress()`, 2-3: `slow_sum(1:2)`, 4-7: `slow_sum(1:4), 8: second `progress()`:
```r
library(progressr)
message("progress_aggregator() ...")
with_progress({
progress <- progressor(steps = 8L)
relay_progress <- progress_aggregator(progress)
progress()
relay_progress(slow_sum(1:2))
relay_progress(slow_sum(1:4))
Sys.sleep(3)
progress()
})
message("progress_aggregator() ... done")
```
Expected result: The progress bar reaches 100% after the `progress()` call in line 11.
Actual result: The progress bar fills quicky, before the second `slow_sum()` even starts.
Reason: Rows like this in `slow_sum()`
https://github.com/HenrikBengtsson/progressr/blob/716d2e750a7667303244f91736f0f8315577e25a/R/slow_sum.R#L24
...together with this line in `R/progress_aggregator.R`:
https://github.com/HenrikBengtsson/progressr/blob/716d2e750a7667303244f91736f0f8315577e25a/R/progress_aggregator.R#L34
The progress call in `slow_sum()` is `progress(amount = 0)` which is captured by the progress aggegator, but that re-emits a progression `progress(child = p)`, omitting the `amount=` option, which then defaults to 1. In fact, no property of the initial progression condition is relayed. The initial progression is just included as an additional option called `child=` but I see that is handled nowhere.
Contributor guide
Assessment
This issue has not been assessed yet.