Use new `sentinel` class for `argparse.SUPPRESS`

[jonathandung]

Feature or enhancement

Proposal:

argparse.SUPPRESS is currently a string but only used in identity comparisons. Though it does get a pretty string representation this way, this is unidiomatic python and causes the constant to have unintended string methods.

I propose to wrap it in a PEP 661 sentinel:

SUPPRESS = sentinel('SUPPRESS')

as opposed to the current:

SUPPRESS = '==SUPPRESS=='

This is a breaking change and may require some discussion. Feedback is much appreciated.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

[ZeroIntensity]

Unfortunately, it looks like people rely on this: https://grep.app/search?q=%3D%3DSUPPRESS%3D%3D

[hugovk]

Yeah, a similar search, excluding argparse.py filenames has 784 results: https://github.com/search?q=%2F%3D%3DSUPPRESS%3D%3D%2F+language%3APython+-path%3A**%2Fargparse.py&type=code

[picnixz]

Since this would break too many projects, I'm closing it now. It would have been good if we were able to, but since people hardcode the SUPPRESS value instead of directly importing, we can't break them like that.

[jonathandung]

I beg to differ. Hardcoding '==SUPPRESS==' is not supposed to work at all. Across the source code of argparse, I found ~15 instances of spam is SUPPRESS or spam is not SUPPRESS, some in public methods. Most notably, these include the implementation of the _parse_known_args2 helper called by the parse_args method, the very purpose of an argument parser. If a literal '==SUPPRESS==' is used, a new string will be created and the is checks should fail, if my understanding is correct. As for the many examples, I don't know why the argumet parsing hasn't acted faulty, but one would only use the argparse.SUPPRESS constant if they are creating and argument parser, which needs the argparse module, so I don't see the point even if it miraculously works.

[jonathandung]

Besides, there are many instances of sentinel being appropriate in the standard library. If this issue remains closed, which is understandable, I shall open a new issue detailing these occurrences.

[picnixz]

Hardcoding '==SUPPRESS==' is not supposed to work at all

Is not supposed to work doesn't mean that it won't. There are projecst that rely on that. We won't break them, it's not worth the burden on either side. It worked well until now.

If a literal '==SUPPRESS==' is used, a new string will be created and the is checks should fail

But that's not the case. 1) you'll get a warning if you use is against string literals, but they do compare equal (at least for interned strings). It's an implementation detail though and may not work on other implementations but breaking CLIs that may have been working until now and are no more maintained is honestly not ideal.

understandable, I shall open a new issue detailing these occurrences.

Don't, there is already an issue open for that, see #149083.