Skip to content

Conversation

@JoelEager
Copy link
Contributor

Per @davidism's request this PR contains my suggested fix for ProgressBar.generator(). In short, this change makes the item_show_func show the item currently being processed as opposed to the item previously processed (like it currently does). I've explained the bug more fully in #1353.

You can use this example to see the difference when run against current master vs this PR.

from time import sleep
from click import progressbar


def format_item(item):
    if item:
        return "processing {}".format(item)


with progressbar(range(5), item_show_func=format_item) as pb:
    for item in pb:
        print(" actually on", item)
        sleep(1)

Thanks in advance for taking the time to look into this.

@davidism
Copy link
Member

davidism commented Mar 4, 2021

This seems incompatible the changes from #1698 which added an update interval. That PR relies on update(), but this one bypasses that and unconditionally calls make_step() and render_progress(). However, moving update() before yield means that the ETA calculation is wrong. Incidentally, that PR is when I decided that the progress bar was getting too complex without enough design oversight and started recommending tqdm.

That said, I do want to get this fix in, I just now need to figure out how to reconcile the two changes.

@davidism davidism added this to the 8.0.0 milestone Mar 4, 2021
@davidism
Copy link
Member

davidism commented Mar 4, 2021

Regarding the ETA issue with this fix, this fix goes against the docs, which causes the ETA and percent complete to be incorrect:

Note that progressbar() updates the bar after each iteration of the loop. So code like this will render correctly:

import time

with click.progressbar([1, 2, 3]) as bar:
    for x in bar:
        print('sleep({})...'.format(x))
        time.sleep(x)

Co-authored-by: David Lord <davidism@gmail.com>
@davidism
Copy link
Member

davidism commented Mar 4, 2021

In order to reconcile the update interval behavior, I changed this back to using self.update(1) after yielding. Before yielding, the progress bar is rendered with the new current item, but only at the start of the interval.

@davidism davidism merged commit 1472e3d into pallets:master Mar 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

f:progress bar feature: progress bar

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProgressBar item_show_func displays the previous item

2 participants