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

How do we feel about adopting rich? #10423

Closed
1 task done
pradyunsg opened this issue Sep 3, 2021 · 13 comments
Closed
1 task done

How do we feel about adopting rich? #10423

pradyunsg opened this issue Sep 3, 2021 · 13 comments
Labels
state: needs discussion This needs some more discussion type: maintenance Related to Development and Maintenance Processes

Comments

@pradyunsg
Copy link
Member

What's the problem this feature will solve?

We're currently using the logging module for handling our output needs. This... works, but is suboptimal since we don't really have any clean way to utilise all the flexibility that a terminal affords us; most notably colors and varying text presentations.

Describe the solution you'd like

Well, part of why I haven't pursued #6541 is that there's no real benefit to doing that without something genuinely beneficial from a UI/UX perspective with that flexibility on the other side.

There weren't any straightforward-and-major presentation gains to be made, by undertaking such a revamp. This is part of why I've let this loiter for so long. With rich though, we would have the required flexibility to actually make meaningful aesthetic improvements available with significantly lower effort (mostly because the required effort was already put into the library).

Alternative Solutions

Maintaining status quo is an option. So, is exploring alternatives to rich that might be more appropriate for our usecase.

I'm wary of directly using something low level though (eg: using colorama directly) since we'd either need to deal with the quirks of the terminal or reimplement many of the features that we'd get with a more rounded-out library like rich.

Additional context

We need to be mindful of our our distribution sizes.

-rw-r--r--  1 pradyunsg  staff   2.6M  3 Sep 10:44 pip-21.3.dev0+rich.tar.gz
-rw-r--r--  1 pradyunsg  staff   1.5M  3 Sep 10:44 pip-21.3.dev0.tar.gz

Most of this ocmes from pygments -- we'll likely need to do something to trim the styles and lexers (~4 MB uncompressed) which we can trim with our vendoring setup. IIUC we don't need any of the web-related styling and only need the lexers related to Markdown and Python.

We'd also need buy-in from the maintainers (/cc @willmcgugan) of rich, since this imposes a couple of restrictions on the library -- it can't adopt non-pure Python dependencies without a graceful fallback and that we're mindful of the sizes of the entire dependency tree.

I had a chat with him a few weeks ago where we talked about this, and he seemed hesitantly excited about the idea. :)

Code of Conduct

@pradyunsg pradyunsg added type: feature request Request for a new feature S: needs triage Issues/PRs that need to be triaged state: needs discussion This needs some more discussion type: maintenance Related to Development and Maintenance Processes and removed S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Sep 3, 2021
@pradyunsg
Copy link
Member Author

Theoretically, we could also just avoid #6541 and just use rich's logging handler: https://rich.readthedocs.io/en/latest/logging.html.

@willmcgugan
Copy link

Author of Rich here. Just to say that if you do decide to go down this route, I would be happy to help.

@pradyunsg
Copy link
Member Author

https://github.com/willmcgugan/rich#rich-library provides a good overview of the capabilities, and https://rich.readthedocs.io/en/latest/markup.html is one of the main draws of the library IMO. :)

@uranusjr
Copy link
Member

uranusjr commented Sep 3, 2021

I’m OK with this as long as I’m not the person doing implementation work. Rich is pretty awesome, but I’d imagine the code base will require some serious refactoring to adopt it. And people running things on non-tty environments (e.g. CI) may not be particularly thrilled.

@pfmoore
Copy link
Member

pfmoore commented Sep 3, 2021

I love rich (thanks for all the work you do on it @willmcgugan!), and I'm broadly in support of using it. I do agree we need to be very careful about bloat, though. We should definitely prune anything we currently vendor that duplicates functionality from rich (colorama, progress) which as @uranusjr says will mean a chunk of refactoring. We should also ideally prune unused functionality from rich, but that's true of all of our vendored libraries, so I'd see this as more of a case of adopting rich having emphasized that we can't keep adding dependencies without dealing with distribution size, and therefore take a hard look at all of our vendored dependencies for what we can trim.

From a quick check, it looks like rich is fine for non-tty output:

❯ type .\test.py
from rich import print
from rich.progress import track
from time import sleep

print("[red]Hello, [blue]world")

for i in track(range(100)):
    sleep(0.1)
❯ py .\test.py >outfile.txt
❯ type .\outfile.txt
Hello, world
Working... ---------------------------------------- 100% 0:00:00
❯

So actually, I'd consider this a benefit of rich (we leave the whole tty or not question to rich).

@pradyunsg
Copy link
Member Author

Rich uses colorama under the hood as well. :)

@pfmoore
Copy link
Member

pfmoore commented Sep 3, 2021

Ah, OK. Nevertheless, if we're going to do this, we should remove any direct use we make of colorama in favour of going through rich's abstractions for everything IMO.

@willmcgugan
Copy link

I wish I could ditch colorama. It was a fine hack at the time, but the new Windows Terminal doesn't need it. Rich will detect the new Windows Terminal (using a method blessed by the developers) and fallback to colorama if needed.

@notatallshaw
Copy link
Member

I wish I could ditch colorama. It was a fine hack at the time, but the new Windows Terminal doesn't need it. Rich will detect the new Windows Terminal (using a method blessed by the developers) and fallback to colorama if needed.

FYI my understanding is the Windows Terminal will be the default command line utility in Windows 11.

At the moment in Windows 10 the user needs to manually download and install the Windows Terminal which in my experience the vast majority of users do not do. Windows 10 consumer support is planned to be ended in 2025.

Sorry if I'm repeating well known information by everyone in this conversation.

@uranusjr
Copy link
Member

uranusjr commented Sep 3, 2021

Not just that, you may be surprised how many people running pip are not even on Windows 10 :p So yeah colorama will stick around for a while.

@pradyunsg
Copy link
Member Author

pradyunsg commented Sep 3, 2021

It's gonna be a while before the vast majority of our users switch to the Windows terminal. That said, I trust @willmcgugan knows this and that whenever he decides its time to remove colorama will be an appropriate time.


We should be able to drop progress and replace it with rich's progress bars. I would expect there to be an overlap time when we vendor them both because it's very likely not going to be a single tiny PR to swap things out. :)

@pradyunsg
Copy link
Member Author

Since no one has said "WHAT ARE YOU SAYING EVEN!? THIS IS A BAD IDEA" to this, I'm gonna go ahead and say that we're OK with doing this.

Time to file a new umbrella issue for tracking actually doing this. :)

@pradyunsg
Copy link
Member Author

#10461 is the follow up.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
state: needs discussion This needs some more discussion type: maintenance Related to Development and Maintenance Processes
Projects
None yet
Development

No branches or pull requests

5 participants