gh-117225: Add color to doctest output #117583
Conversation
|
I think we should also change the British spelling 'colour' to 'color' in the PR's title, which will become the commit message after merging. |
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
This reverts commit bb591b6.
hugovk
changed the title
|
What about also colorising the divider line that's used to separate failed examples? So it would look like this? It might help to see more clearly where one failed example ends and another begins.
This patch does it: diff --git a/Lib/doctest.py b/Lib/doctest.py
index 00ff1a107c..80d0b70592 100644
--- a/Lib/doctest.py
+++ b/Lib/doctest.py
@@ -1307,7 +1307,10 @@ def report_unexpected_exception(self, out, test, example, exc_info):
'Exception raised:\n' + _indent(_exception_traceback(exc_info)))
def _failure_header(self, test, example):
- out = [self.DIVIDER]
+ red, reset = (
+ (ANSIColors.RED, ANSIColors.RESET) if can_colorize() else ("", "")
+ )
+ out = [f"{red}{self.DIVIDER}{reset}"]
if test.filename:
if test.lineno is not None and example.lineno is not None:
lineno = test.lineno + example.lineno + 1
@@ -1621,7 +1624,7 @@ def summarize(self, verbose=None):
print(f" {green}{count:3d} test{s} in {name}{reset}")
if failed:
- print(self.DIVIDER)
+ print(f"{red}{self.DIVIDER}{reset}")
print(f"{red}{_n_items(failed)} had failures:{reset}") |
|
Of these three summary lines at the end: You currently have all three colored in red if there are errors. I think I might be inclined to keep the last line colored in red (as you have it), but not colorize the first two of those lines. I feel like having everything in red makes it harder to read, and having the last line in red is enough to draw your attention to it and communicate "bad!". What do you think? |
|
I've added red divider lines. Edit: need to update tests to disable colour locally.
Can you share a screenshot and a diff to trigger it? I don't get all in red: Screenshots
|
Screenshot in #117583 (comment). For how to trigger it — I was testing by doing some silly things to some doctests in |
|
It looks like many tests fail if you run tests with
Since there are so many that already fail with that environment variable set, I'm not sure it's really an issue, but thought I'd mention it anyway. |
…rom test_traceback to test__colorize
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
|
I was pleased to see that The minor improvement that could be done is to use stub/other way to return empty colors so instead of this: if can_colorize():
bold_green = ANSIColors.BOLD_GREEN
bold_red = ANSIColors.BOLD_RED
green = ANSIColors.GREEN
red = ANSIColors.RED
reset = ANSIColors.RESET
yellow = ANSIColors.YELLOW
else:
bold_green = ""
bold_red = ""
green = ""
red = ""
reset = ""
yellow = ""it could be something like this: ANSIColors = get_ansi_colors()
bold_green = ANSIColors.BOLD_GREEN
bold_red = ANSIColors.BOLD_RED
green = ANSIColors.GREEN
red = ANSIColors.RED
reset = ANSIColors.RESET
yellow = ANSIColors.YELLOW |
Lib/_colorize.py
Outdated
| RED = "\x1b[31m" | ||
| RESET = "\x1b[0m" | ||
| YELLOW = "\x1b[33m" | ||
|
|
There was a problem hiding this comment.
| ANSIColorsStub = type('ANSIColorsStub', (ANSIColors,), {x: "" for x in dir(ANSIColors) if not x.startswith('__')}) | |
| def get_ansi_colors() -> ANSIColors: | |
| return ANSIColors if can_colorize() else ANSIColorsStub |
Using get_ansi_colors() preserves IDE suggestions (ctrl+space) and centralizes, as well as abstracts color availability logic, improving code's clarity and flexibility.
|
Thanks for the suggestion, something like that sounds good. I'll wait to give #117672 a change to be merged first so as not to cause conflicts there, then will update this. I might include the refactoring as part of this PR, but mainly want to make sure this is merged before the beta cutoff on 2024-05-07 so it can be in 3.13. I'll also add some docs, but they can be another PR and can be after the beta cutoff, if necessary. |
|
Plan C: it's been a week and #117672 isn't merged yet. So in this PR, I've moved the colourise functionality back to the |
| GREEN = "\x1b[32m" | ||
| BOLD_GREEN = "\x1b[1;32m" |
There was a problem hiding this comment.
Oh, is there a reason why these colours might have been omitted from the original changes to colourise Python's tracebacks in general? Maybe to avoid accessibility issues for people who are red-green colourblind? Or to avoid issues for people who have light themes on their terminals?
There was a problem hiding this comment.
I think because when there's a traceback error there's nothing "good" to report in green :)
|
The widely-used pytest (116m monthly downloads) uses the same ANSI colours: @encukou made a good point in python/cherry-picker#120 (comment):
We're sticking to the 16 ANSI colours and have @pablogsal How does the following look for you? Is the text still legible? 
|
|
The doctest output looks like:
I moved the existing
_ANSIColorsclass and_can_colorizefunction fromtraceback.pyadded in #112732, to re-use them, cc @pablogsal.Is
_colorizean okay name for this module?If the module has leading underscore, do we also want leading underscores in
_ANSIColorsand_can_colorize?