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

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

Open
mlell opened this issue Nov 12, 2021 · 0 comments
Open

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

mlell opened this issue Nov 12, 2021 · 0 comments

Comments

@mlell
Copy link

mlell commented Nov 12, 2021

The example is copied from 716d2e: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()`:

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.

mlell added a commit to mlell/progressr that referenced this issue Nov 12, 2021
When the progression condition that is to be relayed was created with
any arguments, they need to be relayed as well when creating a new
progression object in progress_aggregator().

Fixes futureverse#126.
mlell added a commit to mlell/progressr that referenced this issue Nov 13, 2021
When the progression condition that is to be relayed was created with
any arguments, they need to be relayed as well when creating a new
progression object in progress_aggregator().

Fixes futureverse#126.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants