futureverse / futureverse/progressr

relay_progress() does not honor progress updates with amount = 0

Open
#126 0 comments 1 reaction 0 assignees View on GitHub
feature/nested-progress
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.