GeezerVison

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?

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.

Issue, NEWS and backport label applied.

Agree this sounds like a bug to me :)

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

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)

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.

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

Sure! Please see PR #128687.

Sure! Please see PR #128687.

Now merged, making this PR much simpler.

Please implement what was discussed above.

Thanks again for the reviews!

Next to update #127877.

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

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

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

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

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

Please see PR #129070 for a fix.