argparse color=False calls _set_color(True) first, then _set_color(False) -- leading to needless envirnment lookups

[hroncok]

Bug report

Bug description:

When debugging another problem (reported in #141570) I realized that in 734e15b, an explicit color argument of self.formatter_class initialization was replaced with a subsequent formatter._set_color(self.color) call. This was done for a good reason -- backwards compatibility.

However, it also leads to unnecessary call of HelpFormatter._set_color(True) in HelpFormatter.__init__, followed by immediate call of HelpFormatter._set_color(color).

• When color is False -- there is a needless lookup for all the environment variables as done in _colorize.can_colorize().
• When color is True -- _colorize.can_colorize() needlessly runs twice.

Perhaps this could be optimized by:

• Replacing HelpFormatter._set_color(color) with HelpFormatter._color = color
• Making self._theme and self._decolor (possibly cached) properties (that change the returned value based on self._color

That way, the code would only run the potentially expensive checks when needed and only once. If this sounds lieka good idea, I can check if it's indeed that simple.

CPython versions tested on:

3.14

Operating systems tested on:

Linux

Linked PRs

• gh-141571: argparse: avoid redundant early _set_color() call in HelpFormatter.__… #141609
• gh-141571: Avoid redundant early _set_color() call in HelpFormatter #141611

[chillakalyan]

I checked the current CPython 3.14 branch and the behavior described in
the bug report no longer exists.

The init() method of HelpFormatter currently contains only a single
call:

self._set_color(color)

There is no earlier call to:

self._set_color(True)

So the double-invocation described in the report is not present in the
latest 3.14 branch. This appears to have been fixed earlier, and no
change is required now.

code where self._set_color() exists:

 def __init__(
         self,
          prog,
          indent_increment=2,
          max_help_position=24,
          width=None,
          color=True,
      ):
          if width is None:
              import shutil
              width = shutil.get_terminal_size().columns
              width -= 2
          // Current 3.14 implementation
          self._set_color(color)

[hroncok]

cpython/Lib/argparse.py

Lines 2725 to 2728 in 4ceb077

	def _get_formatter(self):
	formatter = self.formatter_class(prog=self.prog)
	formatter._set_color(self.color)
	return formatter

This first initializes the formatter class instance with implicit color=True from:

cpython/Lib/argparse.py

Line 169 in 4ceb077

color=True,

That calls _set_color(True) via:

cpython/Lib/argparse.py

Line 177 in 4ceb077

self._set_color(color)

Only to call _set_color(self.color) right after the initialization.

[efimov-mikhail]

@chillakalyan, maybe it'll be helpful for you to read some pages at DevGuide: https://devguide.python.org/getting-started/pull-request-lifecycle/

[mpkocher]

I was debugging a related issue and found the number of calls to can_colorize and isatty to be quite surprising.

The example below with --version or --help will generate 50 calls to isatty and can_colorize.

The origin of the issue is from all of these .get_formatter calls. These are triggered all of over the place. For example, every single call to .add_argument(...) will trigger .get_formatter which will trigger a isatty call.

While this isn't a noticeable performance issue with many applications, it doesn't look great.

Minimal example:

import cProfile
import sys
from pathlib import Path
from argparse import ArgumentParser


def get_parser() -> ArgumentParser:
    p = ArgumentParser(color=True)
    p.add_argument('-v', '--version', action='version', version="0.1.0")

    sp = p.add_subparsers()

    sp1 = sp.add_parser("alpha")
    f = sp1.add_argument
    f('src', help="Path to Input file", type=Path)
    f('-f', '--filter-mode', choices=["alpha", "beta"], help="Fiter modes")
    f('-m', '--max-records', help="Max Records to process", default=10)

    sp2 = sp.add_parser("beta", help="Beta help", description="Beta desc for doing stuff")
    f = sp2.add_argument

    f('src', help="Path to Input file", type=Path)
    f('-f', '--filter-mode', choices=["alpha", "beta"], help="Fiter modes")
    f('-m', '--max-records', help="Max Records to process", default=10)
    f('-n', '--node', help="Node to run on", type=int, default=1)
    return p


def run(argv: list[str]):
    p = get_parser().parse_args(argv)
    print(p)


if __name__ == "__main__":
    cProfile.run("run(['-v'])")
    #run(sys.argv[1:])