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

Fix duplicate progress output in Jupyter #2804

Merged
merged 4 commits into from Mar 4, 2023

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Feb 12, 2023

Type of changes

  • Bug fix
  • New feature
  • Documentation / docstrings
  • Tests
  • Other

Checklist

  • I've run the latest black with default args on new code.
  • I've updated CHANGELOG.md and CONTRIBUTORS.md where appropriate.
  • I've added tests for new code.
  • I accept that @willmcgugan may be pedantic in the code review.

Description

#2449 caused FORCE_COLOR to forcefully imply a terminal. This environment variable tends to be set in a Jupyter context. When Console.is_terminal is True, duplicate output is produced by progress bars (#2740). I haven't tracked down exactly what output when is_terminal=True causes the duplicated output (likely something involving carriage returns or control characters, which Jupyter doesn't always get right), but the fact remains that if you're in a Jupyter kernel, you aren't in a terminal.

This PR shifts the handling of FORCE_COLOR away from _force_terminal and into is_terminal, where other interactive environment detection happens (idle).

Sample output from b89d036:

Screenshot 2023-02-13 at 00 00 52

closes #2740

@minrk
Copy link
Contributor Author

minrk commented Feb 12, 2023

FWIW, the relevant code is in calls to show_cursor. Disabling the show_cursor call (hardcoding the method to return False) fixes the output.

It's specifically a problem with terminal + jupyter, because Jupyter handles the output produced with force_terminal=True just fine, but not when the output widget is involved.

I still don't precisely know why the show_cursor calls trigger OutputWidget thinking there's a newline. This may have to do with the lifecycle of the OutputWidget's capture and when exactly show_cursor is called. I'm not sure. It may also be a bug in OutputWidget's handling of control characters and/or \r.

import time

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

with Progress(console=Console(force_terminal=False, force_jupyter=True)) as p:
    for i in p.track(range(10), description="jupyer, no terminal"):
        time.sleep(0.01)

with Progress(console=Console(force_terminal=True, force_jupyter=False)) as p:
    for i in p.track(range(10), description="terminal, no jupyter"):
        time.sleep(0.01)

with Progress(console=Console(force_terminal=True, force_jupyter=True)) as p:
    for i in p.track(range(10), description="jupyter+terminal"):
        time.sleep(0.01)

produces duplicate output only for jupyter+terminal:

jupyer, no terminal ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
terminal, no jupyter ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
jupyter+terminal ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00
jupyter+terminal ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:00

@minrk minrk changed the title Fix duplicate output in Jupyter Fix duplicate progress output in Jupyter Feb 13, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 4, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (1876380) 98.38% compared to head (178bbb9) 98.38%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2804   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files          74       74           
  Lines        7930     7932    +2     
=======================================
+ Hits         7802     7804    +2     
  Misses        128      128           
Flag Coverage Δ
unittests 98.38% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
rich/_export_format.py 100.00% <ø> (ø)
rich/cells.py 100.00% <ø> (ø)
rich/segment.py 98.72% <ø> (ø)
rich/console.py 98.04% <100.00%> (+<0.01%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@willmcgugan
Copy link
Collaborator

Thanks

@willmcgugan willmcgugan merged commit f2ca04c into Textualize:master Mar 4, 2023
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.

[BUG] Double rendering of notebook output with rich>=12.6.0
3 participants