verigak / progress

Easy to use progress bars for Python
ISC License
1.41k stars 179 forks source link

Update avg/eta/eta_td only once per second #61

Closed pquentin closed 5 years ago

pquentin commented 5 years ago

When the progress bar is updated frequently (every few milliseconds), the eta can change so quickly that it's impossible to read. This is what happens with pip, for example.

This change means we're calling monotonic/time often, but those calls take less than 100 nsec on Linux, Windows and macOS, which is equivalent to one attribute lookup or two.

Oxicode commented 5 years ago

+1 PR

verigak commented 5 years ago

@pquentin You moved the calculation of self.avg outside the if n > 0 block. Can you elaborate why? Also now there is a delay of 1 sec to get the first avg/eta, would it make sense to make an exception for the first second?

praiskup commented 5 years ago

It is IMO somewhat related to this commit https://github.com/python-progress/python-progress/commit/19fd1341594549b3eb6176c7d259f4a385588cbe

pquentin commented 5 years ago

@praiskup it is related, but I tried to send a more minimal fix

@verigak Nice catches! I now update the average while sma_window is being filled, then after every second. I think it's a nice compromise. What do you think?

verigak commented 5 years ago

Thanks, that looks good! Can you squash the last 3 commits into one?

praiskup commented 5 years ago

@pquentin ok, but this does't fix the broken algorithm/miscalculation, right? Check carefully whether the progress bar reports a "correct" speed (or whether it just makes it a bit readable).

pquentin commented 5 years ago

@praiskup I did not read #24 in detail, but as far as I can tell the simple moving average algorithm is correct, the issue is that progress only uses the latest ten updates, and with pip each update is about 10kB. The output could be more precise with an exponential moving average or by using more samples, but that's a different problem in my opinion.

@verigak Thanks, done!

verigak commented 5 years ago

Thanks!