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

[BUG] Rich shouldn't explode when using more than one display #2712

Open
2 tasks done
domef opened this issue Dec 23, 2022 · 6 comments
Open
2 tasks done

[BUG] Rich shouldn't explode when using more than one display #2712

domef opened this issue Dec 23, 2022 · 6 comments

Comments

@domef
Copy link

domef commented Dec 23, 2022

Describe the bug

If you use more than one display, rich raises an exception (rich.errors.LiveError: Only one live display may be active at once).

A minimal example:

from rich.progress import track


for _ in track(range(100)):
    for _ in track(range(100)):
        pass

My horrible fix:

from rich.progress import track


def safe_track(it):
    from rich.errors import LiveError

    it_ = track(it)
    try:
        yield from it_
    except LiveError:
        yield from it


for _ in track(range(100)):
    for _ in safe_track(range(100)):
        pass

I think it shouldn't raise an exception but at most raise a warning. Here I list some reasons:

  • Who uses the iterator shouldn't know if the iterator is decorated with rich or not. If I'm using an iterator from an external library it shouldn't break my program just because it is decorated with rich.
  • It violates the Liskov substitution principle. The iterator decorated with rich has stronger pre-conditions than the original iterator, in fact it requires that no other live displays are currently active. It mean that it can't be used interchangeably with the original iterator.
@github-actions
Copy link

Thank you for your issue. Give us a little time to review it.

PS. You might want to check the FAQ if you haven't done so already.

This is an automated reply, generated by FAQtory

@LucaBonfiglioli
Copy link

I think this issue should at least get some consideration: this is a serious design flaw that hinders the possibility of using rich package to create any reasonably robust library/application.

Think about it: every time I depend on any package that may use rich for whatever reason, I must preventively check that no rich.Live is used and/or monkeypatch it to avoid LiveErrors, but this should not be my responisibility, if a dependency decides, for whatever reason, to use a rich.Live, it should have absolutely no impact on client code and shouldn't have anything to do with it.

@domef
Copy link
Author

domef commented Feb 17, 2023

Any news about this issue? I'm really struggling with this bug. Almost all my repositories that use rich have this problem and I have to scatter that ugly monkeypatch.

@LucaBonfiglioli
Copy link

LucaBonfiglioli commented Feb 22, 2023

I am switching back to TQDM, uglier but much more reliable. No one needs fancy graphics if the underlying software is rotting. Clearly rich is something one should avoid.

@willmcgugan
Copy link
Collaborator

@LucaBonfiglioli If your only contribution to Open Source is to complain then we're not going to miss you. Best of luck.

@tfardet
Copy link

tfardet commented Feb 7, 2024

I've encountered this issue too and, after looking into the code, I'm wondering why the code does not use get_console() and the console's _live attribute as a default when it already exists.
Doing so in my code seems to work fine at the moment.

Sure, the configuration of the existing live and the requested configuration of the new progress may not match, but at the moment there does not seem to be any other obvious way to fix rich, and not being usable out-of-the-box across libraries is a pretty major issue...

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

4 participants