Introduce DiagnosticPipError and use it "once"

[pradyunsg]

Okay, I admit, I nerd-sniped myself into this one. There's decently elaborate commit messages on this one. If you're just here to see how the diagnostic errors might look, see the two test_complete methods, on the test class.

---

This makes a whole lot of progress toward #10421.

• Implements a framework for what I've described in Add additional diagnostics to our error messages #10421 (comment).
• Uses it to add diagnostics to something that does the job of demonstrating how these errors could be implemented and tested. :)

I'd like to get some 👀 on this for gathering feedback on the "overall shape" this is taking.

[DiddiLeija]

This looks fine for me! I just noticed that an (unrelated?) test failed (tests/functional/test_requests.py::test_timeout):

E       assert "Could not fetch URL https://pypi.org/simple/initools/: connection error: HTTPSConnectionPool(host='pypi.org', port=443): Max retries exceeded with url: /simple/initools/ " in "Using pip 21.3.dev0 from /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-1/popen-gw0...6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-1/popen-gw0/test_timeout0/workspace/tmp/pip-req-tracker-v_hdwm41'\n"
E        +  where "Using pip 21.3.dev0 from /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-1/popen-gw0...6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-1/popen-gw0/test_timeout0/workspace/tmp/pip-req-tracker-v_hdwm41'\n" = <tests.lib.TestPipResult object at 0x10a9a1450>.stdout

tests/functional/test_requests.py:14: AssertionError

[sbidoul]

I only skimmed the PR but this looks very nice !

As the transition period may be long, is there a need to pay attention that legacy errors and new-style exceptions render coherently ?

[pradyunsg]

I'd like to get some 👀 on this for gathering feedback on the "overall shape" this is taking.

Nudge for 👀 and green ticks. 😅

[sbidoul]

This looks good to me. The last thing I wanted to do is compare the display of old and new style errors.

[pfmoore]

I like this in principle. My biggest reservation is how this will look on the various terminals people use, and how it degrades in cases where people have limited fonts, or terminals with rendering glitches. That's not something we can test for, it really needs people eyeballing the output on different devices. It might be nice to add a little "driver" script to generate some sample output so that people can see what it looks like.

For example, the "warning sign" emoji renders too wide in the github UI using my browser (Firefox on Windows 10) making following text look misaligned:

Also, that same character renders incorrectly in gvim on Windows:

I don't think either of these is particularly bad, especially as only developers will see them, but if I print the "fancy" output on a default Windows cmd prompt (not using a modern terminal like Windows Terminal, just the standard console host) I get this:

There's only so much we can do about this, but we may want to be conservative, given that a lot of Python users these days are not necessarily familiar with the command line, and are using Windows. (Hmm, has anyone tested how this looks in things like Jupyter using !pip install xxx).

I feel sad that I'm arguing against this - I like the look, and I'm personally using tools capable of handling the Unicode characters just fine (and I'm knowledgeable enough to know what's happening if they go weird on me). But we're very much into UX/UI stuff here, and I fear that friendly but garbled diagnostics may actually make things worse for a non-trivial part of our user base. Do we have anyone with access to users in the newcomer or non-technical communities who can get some real feedback, rather than just my theorising?

[pradyunsg]

I'm painfully aware that the fancy output will need to be gated behind "does the user have a reasonably modern terminal" check, and definitely intend to be mindful of those use cases. I might end up porting over https://github.com/zkat/supports-unicode/blob/main/src/lib.rs, to add that check in our terminal output stuff (which might make sense anyway) -- but we'll cross that bridge when we get there.

For now though, I've not actually used the "fancy" output in anything that pip actually prints out -- rather, the logging infrastructure is using the plain string styling (no reference block, no unicode characters etc. See the except DiagnosticPipError block for the details). The fancy output is implemented because I definitely want to use it eventually, but I definitely don't want to spew out something that someone's stupid terminal can't understand. :)

[pradyunsg]

I think I'm gonna need to get a Windows VM, or a Windows machine. >.<

Update: https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/ -- yay new Microsoft that is actually pro-developer!

[pfmoore]

I think I'm gonna need to get a Windows VM, or a Windows machine

😄 You will be assimilated...

If it's relatively easy-to-reproduce stuff, I'm happy to test things. Feel free to ping me. The more specific, the more likely I'll "just do it" - vaguer things like PR reviews or actually doing any coding are unfortunately being delegated at the moment to the mythical "when I have some spare time" period in the far future 🙁

[pradyunsg]

Alright, if the only concerns are that the fancy output will need sanity checks… it’s not used in the output yet.

I don’t know how to trigger the specific error whose messaging I’ve tweaked here. 😅 I thought it’d be an easy one, but it’s being surprisingly annoying to figure out how to trigger this.

I’ll pick up an easier to test example for the next one, and I’m sure we’ll have time to iterate and improve the messages/format as we add more of these.

[pradyunsg]

Alrighty. Updated this PR to add a few easy-to-test errors.

Before:

❯ pip install ./tests/data/src/pep518_invalid_requires
Processing ./tests/data/src/pep518_invalid_requires
ERROR: file:///Users/pradyunsg/Developer/pip/tests/data/src/pep518_invalid_requires has a pyproject.toml file that does not comply with PEP 518: 'build-system.requires' is not a list of strings.
❯

After:

❯ pip install ./tests/data/src/pep518_invalid_requires
Processing ./tests/data/src/pep518_invalid_requires
ERROR: Can not process file:///Users/pradyunsg/Developer/pip/tests/data/src/pep518_invalid_requires

This package has an invalid `build-system.requires` key in pyproject.toml.
It is not a list of strings.

Note: This is an issue with the package noted above, not pip.
Hint: See PEP 518 for the detailed specification.

❯

Before:

❯ pip install ./tests/data/src/pep518_invalid_build_system 
Processing ./tests/data/src/pep518_invalid_build_system
ERROR: file:///Users/pradyunsg/Developer/pip/tests/data/src/pep518_invalid_build_system has a pyproject.toml file that does not comply with PEP 518: it has a 'build-system' table but not 'build-system.requires' which is mandatory in the table
❯

After:

❯ pip install ./tests/data/src/pep518_invalid_build_system 
Processing ./tests/data/src/pep518_invalid_build_system
ERROR: Can not process file:///Users/pradyunsg/Developer/pip/tests/data/src/pep518_invalid_build_system

This package has an invalid pyproject.toml file.
The [build-system] table is missing the mandatory `requires` key.

Note: This is an issue with the package noted above, not pip.
Hint: See PEP 518 for the detailed specification.

❯

[edmorley]

Thank you for working on this!

[Abdur-rahmaanJ]

I have been using Python on Windows for some 6 years and i twerked my setup to be as good as it get. The best setup i found was ConEmu with git bash as default console.

I can say that as a developer, Windows is a huge pain. If ever we are to rely on emojis appearing on the command prompt of Windows, i can say that we might be effectively waiting till doomsday for that to happen. And as such we are missing big on user experience. Point is i feel that we should disregard the command prompt as a criteria.

2nd is that windows has WSL now which is becoming the standard as for example docker for windows does not run without it. Wsl should give you a near 98% of what Linux terminals are. You kind of no longer need windows-only tools, just use WSL. Icons and more are great. here is something to look at

@pradyunsg I think this is something worth exploring as i don't see any windows version not having WSL in the future. If a win user is reading this, your best bet as a developer is to switch to Linux, with something like Linux Mint to dual boot. You get so much to grow as a developer.

[pradyunsg]

I don’t think there’s any reason to berate Windows as a development platform or to treat it as a second class OS. I’ve noticed that even certain Linux distros and terminal on MacOS also struggle with emojis. As for Windows, the Windows Terminal ships as default in Windows 11, if I understand correctly and it is also a part of the build for the developer-focused Evaluation VM images I linked to above. Windows has a significantly nicer experience than other platforms, if you have the right tools, and I don’t think that pip will go down the route of treating users sub-optimally; just because of the console host they’re running.

I’m leaning toward dropping the emojis FWIW — I don’t like that they don’t have a consistent presentation style across platforms and font settings especially on Linux. I’d prefer to do that stuff in a follow up PR, because this one’s already getting a bit too long and I don’t wanna be changing too many things in one go. :)

[Abdur-rahmaanJ]

Windows has a significantly nicer experience than other platforms, if you have the right tools

For development, Python specifically i think that's not true. It's a real pain. Take CPython dev for example. Yes windows is great to develop on ... to improve windows support.

I don’t think there’s any reason to berate Windows as a development platform or to treat it as a second class OS

The thing is that many dev happen on unix-based systems and as such Windows is bound not to be in phase with many projects without directed efforts. Even then, the command prompt is not the nicest terminal around, by far.

The best thing is to intelligently detect emoji support in the terminal the output is redirecting to. Then output with or without emojis accordingly. Maybe that's difficult to do.

[pradyunsg]

Alrighty. This is pretty much ready to review/merge IMO.

pip install ./tests/data/src/pep518_invalid_build_system is how you'd want to test this, if you have a local clone.

• Trimmed the example change, that we annoyingly difficult to reproduce for testing.
• Trimmed the parts of this PR that weren't being used and were leading to... uhm... distracting discussion (specifically the fancy presentation, with unicode and emojis). Those aren't a concern right now, wherein I mostly just wanna confirm that we're on board for doing this.
• Dropped the assertions for the statements ending in non-alphabetical characters -- it breaks path navigation on terminals that auto-detect paths (uhm... my VS Code environment). I think the _stmt is sufficient to indicate that something should definitely be a full sentence, punctuation and all.

In other news, I prototyped a bit more with the presentation for this, using rich, and tweeted about that here: https://twitter.com/pradyunsg/status/1450573310308364294

---

I'd really like to get this merged soon, so that I can start iterating more on this; to start figuring out the details of the fancier presentation as well as get more of the actual error message improvements filed as PRs. :)

[pradyunsg]

Reading the discussions again today, it seems to me that there's reasonable concensus that we want to do something like this.

I'm gonna go ahead and merge this, to serve as the first iteration for these error message improvements -- I'll keep iterating on this as we go, and I'm happy to incorporate more feedback as it comes in. Please do feel free to suggest improvements / raise concerns in #10421. :)