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

Reposition bars below a closed bar #1502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mjpieters
Copy link

@mjpieters mjpieters commented Aug 24, 2023

When a bar is closed it is moved to pos 0, which means all the other
bars below it must now be given a new position number to have their
output still go to the same screen line.

Fixes #1305, #1331, #1496

  • I have marked all applicable categories:
    • exception-raising fix
    • visual output fix
    • documentation modification
    • new feature
  • If applicable, I have mentioned the relevant/related issue(s)

Less important but also useful:

  • I have visited the source website, and in particular
    read the known issues
  • I have searched through the issue tracker for duplicates
  • I have mentioned version numbers, operating system and
    environment, where applicable:
    import tqdm, sys
    print(tqdm.__version__, sys.version, sys.platform)

tqdm/std.py Outdated
Comment on lines 1322 to 1331
finally:
# Remove from internal set
self._decr_instances(self)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to a finally block to first adjust the closed bar to the top row, before other bars adjust their position upwards. This results in a cleaner screen update, I think.

@mjpieters
Copy link
Author

The reproducer demo from #1496 looks like this when using the changes in this PR:

Screen.Recording.2023-08-24.at.18.46.48.mov

@fireattack
Copy link

fireattack commented Aug 25, 2023

I tried with a simple sync case, leave=None and leave=False doesn't look right.

Test code:

from tqdm import tqdm
import time
import random

for leave in [True, None, False]:
    print('Leave: ', leave)
    print('-------------------')

    bars = [tqdm(total=1024, unit="B", unit_scale=True, leave=leave, unit_divisor=1024, ncols=100, position=i) for i in  range(0, 5)]
    size = int(1024*1024*10*random.random())
    for i, bar in enumerate(bars):
        bar.desc = f'Bar {i + 1}/5'
        bar.reset(total=size)

    offset = 0
    while True:
        if offset < size:
            update_chunk = 1024*200
            time.sleep(.1)
            offset += update_chunk
            if offset > size:
                update_chunk -= offset - size
            for bar in bars:
                bar.update(update_chunk)
        else:
            break

    for bar in bars:
        bar.close()

    print('Done!')
    print('-------------------')

Output with 4.66.1 release:

>temp
Leave:  True
-------------------
Bar 1/5: 100%|█████████████████████████████████████████████████| 1.76M/1.76M [00:01<00:00, 1.82MB/s]
Bar 2/5: 100%|█████████████████████████████████████████████████| 1.76M/1.76M [00:01<00:00, 1.82MB/s]
Bar 3/5: 100%|█████████████████████████████████████████████████| 1.76M/1.76M [00:01<00:00, 1.82MB/s]
Bar 4/5: 100%|█████████████████████████████████████████████████| 1.76M/1.76M [00:01<00:00, 1.82MB/s]
Bar 5/5: 100%|█████████████████████████████████████████████████| 1.76M/1.76M [00:01<00:00, 1.82MB/s]
Done!
-------------------
Leave:  None
-------------------
Bar 1/5: 100%|█████████████████████████████████████████████████| 6.27M/6.27M [00:03<00:00, 1.97MB/s]
Bar 2/5: 100%|████████████████████████████████████████████████▊| 6.25M/6.27M [00:03<00:00, 2.02MB/s]Done!
-------------------
Leave:  False
-------------------
                                                                                                    Done!
-------------------

Output with tqdm-4.66.2.dev2+gbf765f3 (this PR):

>temp
Leave:  True
-------------------
Bar 1/5: 100%|█████████████████████████████████████████████████| 7.99M/7.99M [00:04<00:00, 2.01MB/s]
Bar 2/5: 100%|█████████████████████████████████████████████████| 7.99M/7.99M [00:04<00:00, 2.01MB/s]
Bar 3/5: 100%|█████████████████████████████████████████████████| 7.99M/7.99M [00:04<00:00, 2.01MB/s]
Bar 4/5: 100%|█████████████████████████████████████████████████| 7.99M/7.99M [00:04<00:00, 2.01MB/s]
Bar 5/5: 100%|█████████████████████████████████████████████████| 7.99M/7.99M [00:04<00:00, 2.01MB/s]
Done!
-------------------
Leave:  None
-------------------
Bar 1/5: 100%|█████████████████████████████████████████████████| 6.95M/6.95M [00:03<00:00, 1.99MB/s]
Bar 2/5: 100%|█████████████████████████████████████████████████| 6.95M/6.95M [00:03<00:00, 1.99MB/s]
Bar 3/5: 100%|█████████████████████████████████████████████████| 6.95M/6.95M [00:03<00:00, 1.99MB/s]
Bar 4/5: 100%|█████████████████████████████████████████████████| 6.95M/6.95M [00:03<00:00, 1.99MB/s]
Bar 5/5: 100%|█████████████████████████████████████████████████| 6.95M/6.95M [00:03<00:00, 1.99MB/s]
Done!
-------------------
Leave:  False
-------------------
Done!
-------------------████████████████████████████████████████████| 6.21M/6.21M [00:03<00:00, 2.02MB/s]
Bar 5/5: 100%|█████████████████████████████████████████████████| 6.21M/6.21M [00:03<00:00, 2.02MB/s]
D:\sync\code\python\_gists>████████████████████████████████████| 6.21M/6.21M [00:03<00:00, 2.02MB/s]
Bar 5/5:  98%|███████████████████████████████████████████████▊ | 6.05M/6.21M [00:03<00:00, 2.02MB/s]


Issues with release:

  • leave=None failed to get rid of bar2. But bar3-5 are indeed gone as expected.
  • leave=False: all bars are gone, but the cursor isn't reset to the left.

Issues with this PR:

  • leave=None seems to have no effect at all. It's the same as leave=True.
  • leave=False now has totally mangled output. Leftover after the bars are closed shown in between other outputs afterwards.

@fireattack
Copy link

Your comment in #1496:

Closed bars are considered outside the scope of screen that active bars are on, so they are moved to position 0

do inspire me a quick fix for my case:

Just do

for bar in reversed(bars):
    bar.close()

And now both the release and your PR works flawlessly

@mjpieters
Copy link
Author

mjpieters commented Aug 25, 2023

Thanks for providing additional tests! This is really helpful.

Issues with this PR:

* leave=None seems to have no effect at all. It's the same as leave=True.

Ah, yes, that'd be a bug because in that case only the first bar should be left. But because my change causes other bars to be moved to position 0 they are now also left when completed. It should be a simple enough change to set leave=False when leave is None and pos was changed to 0.

* leave=False now has totally mangled output. Leftover after the bars are closed shown in between other outputs afterwards.

Ah, when moving up bars they now leave a ghost. I added a clear(nolock=True) to remedy that.

Here is my updated reproducer, using all three modes:

import asyncio
from tqdm.asyncio import trange

async def bar(pos: int, name: str, colour, delay: float, leave: bool | None):
    for _ in trange(10, desc=name, colour=colour, position=pos, leave=leave, postfix={"pos": pos}):
        await asyncio.sleep(delay)

async def main():
    bars = [("A", "green", 0.1), ("B", "cyan", 0.6), ("C", "yellow", 0.25), ("D", "magenta", 0.4)]
    for leave in (None, False, True):
        print("Leave:", leave)
        tasks = [bar(pos, *args, leave) for pos, args in enumerate(bars)]
        await asyncio.gather(*tasks)
        print("Done!")

asyncio.run(main())

and the output this produces:

Screen.Recording.2023-08-25.at.14.40.25.mov

I did notice that there is a race condition that affects one of the tests I added, test_position_leave[True], where there is a ANSI CSR Cursor Up entry missing:

E   AssertionError: Got => Expected
E   '\n\n\rpos2 bar:  50%|#####     | 1/2 [00:00<00:00, 10082.46it/s] \x1b[A' => '\n\n\rpos2 bar:  50%'

but I can't figure out why this would be the case as every moveto(pos) call is paired with moveto(-pos) with pos being a local variable, plus the lock is being held at the time so nothing should be interfering here. It happens more often when there is more than one test running

When a bar is closed it is moved to pos 0, which means all the other
bars below it must now be given a new position number to have their
output still go to the same screen line.
@sheindel-llt
Copy link

Is this fix still in progress? Would be greatly appreciated as the 'ghost' progress bars cause a lot of user confusion. I am unable to help with development but could test if that is helpful.

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

Successfully merging this pull request may close these issues.

Random duplicated line on multithreaded progress bard update
3 participants