gh-128595: Default to stdout isatty for colour detection instead of stderr

[hugovk]

As suggested at #128317 (comment), default to checking stdout's isatty rather than stderr's.

Also using the @force_not_colorized_test_class decorator from #127877 to run some tests without worrying about colour.

Before	After

• Issue: Default to stdout isatty for colour detection instead of stderr #128595

[hugovk]

Should this be considered a bugfix to backport to 3.13, with its own issue and NEWS, or piggyback on gh-128317 for 3.14 only?

[erlend-aasland]

Should this be considered a bugfix to backport to 3.13, with its own issue and NEWS, or piggyback on gh-128317 for 3.14 only?

I would say it deserves an issue and a NEWS entry. I'm fine with adding it to #128317 if it's not going to be backported. I'm leaning into calling it a bugfix.

[hugovk]

Issue, NEWS and backport label applied.

[zanieb]

Agree this sounds like a bug to me :)

[serhiy-storchaka]

Why not check this for stdout and stderr separately? If the traceback is written to terminal, it should be colorized.

[hugovk]

Why not check this for stdout and stderr separately? If the traceback is written to terminal, it should be colorized.

For example, something like this?

-def can_colorize() -> bool:
+def can_colorize(file=sys.stdout) -> bool:
     if not sys.flags.ignore_environment:
         if os.environ.get("PYTHON_COLORS") == "0":
             return False
@@ -49,7 +49,7 @@ def can_colorize() -> bool:
         if os.environ.get("TERM") == "dumb":
             return False
 
-    if not hasattr(sys.stdout, "fileno"):
+    if not hasattr(file, "fileno"):
         return False
 
     if sys.platform == "win32":
@@ -62,6 +62,6 @@ def can_colorize() -> bool:
             return False
 
     try:
-        return os.isatty(sys.stdout.fileno())
+        return os.isatty(file.fileno())
     except io.UnsupportedOperation:
         return sys.stdout.isatty()

And then places like traceback.py can do something like this?

 def _print_exception_bltin(exc, /):
     file = sys.stderr if sys.stderr is not None else sys.__stderr__
-    colorize = _colorize.can_colorize()
+    colorize = _colorize.can_colorize(file=file)
     return print_exception(exc, limit=BUILTIN_EXCEPTION_LIMIT, file=file, colorize=colorize)

[serhiy-storchaka]

Yes, except that dynamic value sys.stdout should not be used as the default value. Use None and then replace it with the current value of sys.stdout inside the function.

[vstinner]

Can you extract the code to disable colors as a separated PR and merge it first?

[hugovk]

Sure! Please see PR #128687.

[hugovk]

Sure! Please see PR #128687.

Now merged, making this PR much simpler.

[serhiy-storchaka]

Please implement what was discussed above.

[vstinner]

LGTM. Thanks for the update (adding file). I just have two minor comments.

[vstinner]

LGTM

[hugovk]

Thanks again for the reviews!

Next to update #127877.

[miss-islington-app]

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @hugovk, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 6f167d71347de6717d9f6b64026e21f23d41ef0b 3.13

[bedevere-app]

GH-129057 is a backport of this pull request to the 3.13 branch.

[picnixz]

Some build bots are now failing (https://buildbot.python.org/api/v2/logs/12166870/raw_inline):

TypeError: setUpModule..() got an unexpected keyword argument 'file'