Use Rich for Printing and Robust Progress/Wait Bars#740
Conversation
|
@rmartin16 I've been meaning to take a closer look at rich, and this gave me as good a reason as any :-) I'm OK with adding rich as a dependency. It's becoming a relatively well-accepted part of the Python ecosystem, so it's not especially controversial to add it; and the API looks relatively straightforward. However, I have some reservations about the specifics - IMHO, some of the default style choices provided by rich are have arguable UX benefits. The progress bar is OK; I'd rather if it was trimmed rather than "full width of terminal" (similar to what pip does). This is a fundamental issue of typography and layout that publications like Bringhurst's "Elements of Typographic Style" have encouraged for a long time. Long lines aren't visually scannable; 40-75 character lines are encouraged, (with 66 generally being considered ideal). The "wait bar" is another matter. I've historically had pretty good eyesight, but I could only barely see the "throbbing" of the icon on the waitbar. I don't know if that's the choice of icon, or the BRIGHT GREEN color that it was using, but I could hardly see that something was happening. That leads to the other major concern I have about introducing Rich - I'm not wild about rich's Colors Everywhere approach. Colors can be very useful. When used well, they can draw attention to important details, or highlight points of concern. I'd be 100% on board with introducing colours for warnings to make them stand out, or using gray/faded colours for some of the app-level annotations (ie. the On top of that, I'm acutely aware of the impact color choices have on people with color blindness (my son is color blind) - or even on people who have white vs black console backgrounds. That said - I'm not opposed to adding colors or other rich-derived UI elements. With some stylistic tweaks, I'm on board. I haven't done enough investigation to see what customisation options exist; and be prepared for me to sweat the minor details as we go down this path. |
|
The Colors Everywhere is real here....they are definitely not being subtle :) It appears, though, to largely be possible to tell Rich to use the "default" color scheme of the terminal (albeit with some exceptions I've found). From reviewing how Rich is structured, here is what I'm thinking; the code is still POC quality...want to agree on design first. Leveraging Rich Markup
Wait/Status Bar
from time import sleep
from rich.console import Console as RichConsole
rich_console = RichConsole(highlight=False)
wait_bar = rich_console.status("Waiting to boot up...", spinner="bouncingBar", spinner_style="default")
with wait_bar:
for idx in range(10):
print(f">>> status update {idx}")
sleep(1)Progress Bar
from time import sleep
from rich.console import Console as RichConsole
from rich.progress import BarColumn, Progress, SpinnerColumn, TextColumn, TimeRemainingColumn
rich_console = RichConsole(highlight=False)
def main():
progress_bar = Progress(
" ",
SpinnerColumn("line", speed=1.5, style="default"),
BarColumn(bar_width=50),
TextColumn("{task.percentage:>3.1f}%", style="default"),
"•",
TimeRemainingColumn(compact=True, elapsed_when_finished=True),
console=rich_console,
)
print("Downloading package...")
total = 10
task_id = progress_bar.add_task("task desc", total=total, start=True)
with progress_bar:
for idx in range(total):
print(f">>> status update {idx}")
progress_bar.update(task_id, advance=1)
sleep(1)
main() |
👍 This all makes sense.
Ironically, the URL highlighting is the one thing I could probably live with, as it matches native HTML stylesheets, and the blue they've chosen isn't as eye-searing as some of the other colour choices. Would a custom theme that allows us to selectively turns on some of the highlighting choices be an option?
Hrm. This seems... potentially explosive. Imagine if a user had an app named It also makes me wonder if we might want to abstract the
The option you've picked looks reasonable enough. My ideal outcome would be something that matches the graphical style of the progress bar - but that doesn't appear to exist out of the box based on that built-in demo. The
😄
🤷 Sure - time remaining will be 100% A Lie (tm), but it's better than a poke in the eye with a sharp stick, and at least gives an indication of how long you have to wait.
MB/s is a fun metric, but not an especially actionable one. If it turns out your download is slow, it's not like you could point it at a different download source. And if the download is slow, that's going to be obvious from the "time remaining: 4 hours" indicator - MB/s won't tell you anything new.
Again - a cute idea, but if we're in a situation where we need multiple progress bars, I think we need to revisit the UX.
That looks fine. If nothing else, it's at least consistent with pip, so it won't be surprising to anyone. |
Selective default formatting:
A custom highlighter can be specified for Printing user content:
Agreed; something will definitely need to accommodate this....I struggled to come up with anything quickly but I'll experiment some more. Wait/status bar graphic:
I mocked up this example below that uses from time import sleep
from rich.console import Console as RichConsole
from rich.progress import BarColumn, Progress
rich_console = RichConsole(highlight=False)
wait_bar = Progress(
BarColumn(bar_width=6, style="black", pulse_style="white"),
"Waiting to boot up...",
transient=True,
console=rich_console,
)
wait_bar.add_task("task desc", start=False)
with wait_bar:
for idx in range(10):
print(f">>> status update {idx}")
sleep(1) |
Selective default formatting Part 2:After further considering how to limit Rich's default coloring, I think it makes more sense to create a custom highlighter class for Rich's I reviewed Rich's default highlighting conditions and summarized them below:
Demonstration: from rich.console import Console
from rich.highlighter import RegexHighlighter
text="\n".join(["\[openTag] \[/closeTag]", "<attrib attrib_var=attrib_val> </attrib>", "1.1.1.1", "20e6:1994::0000", "0cf0d5e0-dd83-41b6-b51b-359a9be54ac2", "klass.method()", "True False", "sigh...", "123 1.23 1+4j", "//europa/music/awesome.mp4", "https://www.google.com/asdf", "'string' \"string\""])
class RichConsoleHighlighter(RegexHighlighter):
base_style = "repr."
highlights = [r"(?P<url>(file|https|http|ws|wss)://[-0-9a-zA-Z$_+!`(),.?/;:&=%#]*)"]
Console().print(text)
Console(highlighter=RichConsoleHighlighter()).print(text)Also, by default, Rich implements soft wrapping to avoid breaking words between lines in the terminal....fyi. |
Printing
|
6cb249f to
071d7d9
Compare
|
OK, I think I've landed on an approach I like. My comments on "selective default formatting" above stand (i.e. use a custom highlighter for the Rich Printing [myapp] Prefixes Part 2The crux of this approach to managing To use markup, calls to As for Progress BarRich's API design for a "progress display" requires users to create a progress bar and add at least one "task" to generate an on-screen progress bar. This isn't especially onerous but it does expose Rich's progress bar implementation anywhere we use a progress bar. If we wanted to avoid this, I could implement Wait BarOn top of the changes to the wait bar I detailed previously, I also added a couple other features.
With all this said, I think that's all the scope of this PR. (For instance, I'll create another PR to implement this new wait bar for opening the iOS simulator). Once we can hash the details for these high level changes, I'll update the rest of the codebase to use it as well as fix all the tests this is gonna break. |
80989f5 to
7d4946a
Compare
|
Apologies for the delayed response; I've been knee deep in Android build system code, and wanted to give your comments the consideration they deserve. tl;dr - I think this is converging on a really good place. It's getting lots of useful aspects of rich without going full technicolor rainbow, and the implementation isn't especially onerous.
+1 to these settings. It would be worth adding a docstring in the code to summarise what we're doing (and why) so future explorers know why it's been done.
+1. This is a nice compromise. It exposes the functionality in case it might be useful, but it won't be "explosive by default".
Makes sense. In practice, I don't think we're likely to use both a prefix and a preface, but it's good to know we've got a clear story. Sweating the little styling details - I wonder if it would make more sense to make the default rendering color for DEBUG/DEEP_DEBUG output light gray as well. When there's a lot of debug output, it still needs to be legible, but it's less important that it's immediately legible; and it's beneficial to be able to pick out the "real" output from the debug output.
I think I can live with the API exposure. It's not too onerous, and the API that is being exposed makes sense in the circumstances.
These changes all look great. I quite like this new black and white wait bar; my only suggestion would be to make it a little wider (say, 40 chars, with a 5 character spacer on the left) so it's thematically consistent with the way we render progress bars like the download bar.
Sounds like a plan. I'm wary of splitting this into too many PRs - but I'm also appreciative that monster PRs take too long to get through the whole process. At the very most, I'd want to see 1 PR for the core functionality, plus one per platform; but if we can get it in less PRs than that (1 for the core, plus 1 to roll it out everywhere), then I'd be OK with that too. |
|
@rmartin16 FYI - I've just merged #744, which applies black to the entire codebase, so there are some merge issues to contend with. You may find |
05bf8a5 to
e7c3286
Compare
I think this is a great idea; it's perfectly legible to me while allowing me to keep my attention on
I'm not sure I entirely follow this; can you please describe the full layout of the wait bar you're imagining? I think my primary question might stem from what to do with the message associated with the wait bar. If the wait bar is 40 characters wide, then putting the message to the right of the bar looks pretty awkward....but it also feels a little weird on the left side. I think you might mean for the message to be on the line above the wait bar....if so, I mocked up what that would take....its a little more sophisticated use of Rich but not too bad after enough playing around with it. And so it's clear, the use of Additionally, I updated all uses of |
9d60b03 to
6c50861
Compare
It's a little hard to render in Github Markdown, but I'm thinking something like: That would then be comparable with the layout of progress bars:
|
4a8f83b to
69323f7
Compare
- Use Rich to print all output to terminal for Rich markup support - Use Rich progress display to implement robust Progress and Wait bars
|
This is everything for us to review in this PR. A few notes. I updated the Wait Bar to match your layout; interested in any thoughts on the other functionality I implemented for the Wait Bar. I did discover an issue with a use-case I've been targeting for the Wait Bar, i.e. wrapping calls to Example of spawned process printing in wait barimport subprocess
import sys
import time
from briefcase.console import Console
cmd = 'from time import sleep\n'\
'print("line 1")\n'\
'sleep(1)\n'\
'print("line 2")\n'\
'print("line 3")\n'\
'sleep(1)\n'\
'print("last line")\n'
def main():
with Console().wait_bar("waiting..."):
r=subprocess.run([sys.executable, "-c", cmd], check=True)
time.sleep(3)
main()I also hit that race condition in the tests for subprocess where |
Codecov Report
|
freakboy3742
left a comment
There was a problem hiding this comment.
This is awesome!
I've made a couple of small tweaks during review:
- I've reduced the final size of the wait bar. After actually living with it, the 40 char wide bar was visually consistent with progress bars, but a little garish. Cutting it down to 20 means there's only one "pulse", but it's still in the same general style as the progress bars,
- I've made the default style for warnings yellow, and the default style for errors red. They're sufficiently rare that you don't normally see them; but when you do, they need to stand out. I think that's a good match for the "gray on debug" style.
- I've added iOS waitbar support. This was partially because I didn't want iOS to be left behind, partially to save you the work (since you've done so much on this already!), and partially so that I could make sure I understood how all the new stuff works. It was... disturbingly simple to implement - which is a testament to the great API you (and, I guess, Will McGuigan) have put together here.
Lastly, I've promoted this to a full feature in the release note. There's no way we're going to add this much polish and slide it under the radar as a "miscellaneous improvement" :-)
I'm really happy with where this has ended up. The style improvements are subtle but significant improvements, without being overbearing. Thanks for doing all the heavy lifting - it's much appreciated!
These changes leverage Rich for all terminal output.
markup=True; this is true for logging as well as input prompting.pipdownload bar.Note: While a progress or wait bar is ongoing, additional output can only be printed via Rich. This means, for instance, a wait bar cannot be used to wrap a subprocess that may output the terminal itself; otherwise, that process' output will awkwardly be combined with the Rich bar.
Original Post Content
A quick and dirty POC for much more robust progress and status bars. Although, a bit of bait and switch since this simply
implements Textualize/rich. As I was assessing what it would take to enhance the existing bars to allow for interspersed printing, it really started feeling like the wrong approach. And something that implemented more of a
curses-like approach would be much more fitting. Although, not using an established library would definitely be a reinvention of the wheel.@freakboy3742, if you're amenable to using rich, I can formalize this PR; please let me know your thoughts.
PR Checklist: