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

Strange behaviors when using rich package in dealing with multiple line representation in Spyder console #494

Open
MekJohn opened this issue Jun 7, 2024 · 7 comments
Labels
type:Enhancement New feature or request
Milestone

Comments

@MekJohn
Copy link

MekJohn commented Jun 7, 2024

Using rich python package all going fine if you use only one line on live console display, but something weird come up when multiple line display are required with other complex objects like progress bars or tables ...

One line progress is ok in the spider console. See the following code took from rich documentation.

import time
from rich.progress import track

for i in track(range(20), description="Processing..."):
    time.sleep(1) 

image

The weird things happen when you have to use multiple lines representation (also this example is taken from rich documentation).

import time

from rich.progress import Progress

with Progress() as progress:

    task1 = progress.add_task("[red]Downloading...", total=1000)
    task2 = progress.add_task("[green]Processing...", total=1000)
    task3 = progress.add_task("[cyan]Cooking...", total=1000)

    while not progress.finished:
        progress.update(task1, advance=0.5)
        progress.update(task2, advance=0.3)
        progress.update(task3, advance=0.9)
        time.sleep(0.02)

In Spider console this last code block displays a result where only the last line is kept updated in place, printing others in multiple lines for each updates. See below.

image

In win console obviously there aren't any problems about that. All the three lines are kept updated in place.

image

Is there a way to solve it?


Win 11
Python 3.11
Spyder IDE 5.4.4
spyder-kernels 2.4.2
PyQt5 5.15.10
PyQt5-Qt5 5.15.2
PyQt5-sip 12.13.0_

@dalthviz
Copy link
Member

Hi there @MekJohn thank you for the feedback! I think this something that needs to be solved over QtConsole. In fact, we are aiming to improve the behavior over jupyter/qtconsole#616. However, seems like we need further work over there. Running your example code with the changes over there I see the following:

image

@dalthviz
Copy link
Member

Quick update to say that after checking the Rich Console API, using jupyter/qtconsole#616 and with the following example code:

import time

from rich.console import Console
from rich.progress import Progress

console = Console(force_jupyter=False)

with Progress(console=console) as progress:

    task1 = progress.add_task("[red]Downloading...", total=1000)
    task2 = progress.add_task("[green]Processing...", total=1000)
    task3 = progress.add_task("[cyan]Cooking...", total=1000)

    while not progress.finished:
        progress.update(task1, advance=0.5)
        progress.update(task2, advance=0.3)
        progress.update(task3, advance=0.9)
        time.sleep(0.02)

I'm able to see the following behavior 🎉 :

progress

@ccordoba12
Copy link
Member

ccordoba12 commented Oct 22, 2024

@dalthviz, I think to solve this we can overload the Rich Console class in our sitecustomize so that force_jupyter=False is set on it by default.

That'd be similar to what we do to patch QApplication in our kernel:

class SpyderQApplication(QtWidgets.QApplication):
def __init__(self, *args, **kwargs):
super(SpyderQApplication, self).__init__(*args, **kwargs)
# Add reference to avoid destruction
# This creates a Memory leak but avoids a Segmentation fault
SpyderQApplication._instance_list.append(self)
SpyderQApplication._instance_list = []
QtWidgets.QApplication = SpyderQApplication

@ccordoba12 ccordoba12 added the type:Enhancement New feature or request label Oct 22, 2024
@ccordoba12 ccordoba12 added this to the v3.1.0 milestone Oct 22, 2024
@dalthviz
Copy link
Member

dalthviz commented Oct 24, 2024

Checking over Rich, maybe what is needed is to change the handling done over _is_jupyter to return False: https://github.com/Textualize/rich/blob/e9f75c9912ed25b9777bc0257853370951220b17/rich/console.py#L518-L535

That is the function that is called when nothing is passed over the Console constructor for force_jupyter: https://github.com/Textualize/rich/blob/e9f75c9912ed25b9777bc0257853370951220b17/rich/console.py#L670

Seems to me that instead of being handle along side Jupyter Notebooks probably QtConsole should be handle with the IPython case (returning False)? So changing that method to be something like:

def _is_jupyter() -> bool:  # pragma: no cover
    """Check if we're running in a Jupyter notebook."""
    try:
        get_ipython  # type: ignore[name-defined]
    except NameError:
        return False
    ipython = get_ipython()  # type: ignore[name-defined]
    shell = ipython.__class__.__name__
    if (
        "google.colab" in str(ipython.__class__)
        or os.getenv("DATABRICKS_RUNTIME_VERSION")
    ):
        return True  # Jupyter notebook
    elif shell in ["TerminalInteractiveShell", "ZMQInteractiveShell"]:
        return False  # Terminal running IPython or QtConsole
    else:
        return False  # Other type (?)

What do you think @ccordoba12 ? Or even if no changes are done over the Rich package side, should _is_jupyter be the thing that gets patched here (over spydercustomize)?

@ccordoba12 ccordoba12 changed the title Strange behaviors when using rich package in dealing with multiple line representation in Spider console. Strange behaviors when using rich package in dealing with multiple line representation in Spyder console Nov 7, 2024
@ccordoba12
Copy link
Member

ccordoba12 commented Nov 7, 2024

Seems to me that instead of being handle along side Jupyter Notebooks probably QtConsole should be handle with the IPython case (returning False)? So changing that method to be something like

Thanks for checking that @dalthviz! Unfortunately we can't add ZMQInteractiveShell in this block

    if ...:
        ...
    elif shell in ["TerminalInteractiveShell", "ZMQInteractiveShell"]:
        return False  # Terminal running IPython or QtConsole

because that will return False for Jupyter notebooks (both QtConsole and JupyterLab use IPykernel as its default kernel, whose shell class is precisely ZMQInteractiveShell).

However, we have our own shell class in Spyder-kernels (SpyderShell), which we could add to that elif block so that _is_jupyter gives False for Spyder.

Since that's a very simple change, I think you can submit a PR to Rich with it and it'll be accepted.

@dalthviz
Copy link
Member

dalthviz commented Nov 7, 2024

Actually, since spyder-kernels uses SpyderShell then I think just by using QtConsole 5.6.1 things should just work when running things with Spyder. In fact, doing a quick check with Spyder 6.0.2 from the windows installer while using a custom interpreter with spyder-kernels (v3.0.0) and rich (v13.7.1) installed, I'm able to see the progress bars working:

imagen

So I guess from the Spyder side things are already working (as long as you have QtConsole 5.6.1 installed)? Maybe for the QtConsole side of things we should just create some documentation related with tips to be able to use Rich or something like that? 🤔

@ccordoba12
Copy link
Member

So I guess from the Spyder side things are already working (as long as you have QtConsole 5.6.1 installed)?

Yeah! That's really nice 👍🏽

Maybe for the QtConsole side of things we should just create some documentation related with tips to be able to use Rich or something like that? 🤔

The problem is that it's not possible for the kernel to tell if it's running code that was sent from QtConsole or JupyterLab (in other words, it's frontend agnostic). And that means that that _is_jupyter function is actually wrong. But since JupyterLab is way more popular than QtConsole, people prefer to resort to those hacky approaches to support notebooks.

We could implement a proper solution in Spyder (to support both Spyder-notebook and the IPython console) by sending to the kernel the current plugin with focus and making _is_jupyter return True or False in our sitecustomize accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants