spyder-ide / spyder-kernels

Jupyter Kernels for the Spyder console
MIT License
39 stars 40 forks source link

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

Open MekJohn opened 5 months ago

MekJohn commented 5 months ago

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 commented 2 months ago

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 https://github.com/jupyter/qtconsole/pull/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 commented 2 months ago

Quick update to say that after checking the Rich Console API, using https://github.com/jupyter/qtconsole/pull/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 :tada: :

progress

ccordoba12 commented 1 month ago

@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:

https://github.com/spyder-ide/spyder-kernels/blob/b6ffbb42b5b531cfea9c0a9fedc94f9548c44f9e/spyder_kernels/customize/spydercustomize.py#L88-L96

dalthviz commented 1 month ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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.