All commit authors signed the Contributor License Agreement.

Replacingrepr withmodule_name is technically a breaking change.

I think breaking changes are expected for draft PEPs, so imo no deprecation process should be used. Users should be aware that changes are expected. However, I think we can:

• Improvethe documentation about draft PEPs, stating that implementation can change.
• Explicitly state that Sentinels are from a draft PEP in theSentinel class documentation, and isn't stable yet.

cc@taleinat

I'm not willing to remove therepr parameter fromtyping_extensions.Sentinel without a deprecation. However, I'm OK to re-export the builtin Sentinel class in 3.15 (if it makes it in), even if it doesn't have this parameter.

I'm not willing to remove therepr parameter fromtyping_extensions.Sentinel without a deprecation. However, I'm OK to re-export the builtin Sentinel class in 3.15 (if it makes it in), even if it doesn't have this parameter.

Then I will changerepr into a keyword parameter.

If compatibility is important enough then I can also have the code assumemodule_name is actuallyrepr if it does not refer to a existing module. Would this be necessary?

Configuration likerepr would be defined only with the initial Sentinel object. It wouldn't get stored in the reduce function. This allows changes inrepr to be reflected in unpickled sentinels. This means that removingrepr later will not break serialization.

I've replaced the__reduce__ method with a version using pickle's singleton support. This is the most conservative and inoffensive option for a new reduce function since it doesn't add a custom unpickle function, is forward compatible with any future method of pickling, is the standard method of handling singletons via pickle, and is generally strict. This does not support pickling anonymous sentinels which will now raisepickle.PicklingError instead ofTypeError. I can revert this if the previous behavior was more desired, but I personally only need to pickle sentinels which have a top-level definition, and the more I work with them the more that anonymous sentinels feel counter intuitive.

I've attempted to add to the discussion of PEP 661. Anonymous sentinels bring a lot of issues which need to be addressed.
https://discuss.python.org/t/pep-661-sentinel-values/9126/251

Before this is merged I'd like to add astrict keyword parameter to Sentinel which prevents sentinels from being accidentally named after existing top-level objects which are not Sentinel as well as ensuring thatrepr is never given conflicting values. I'll need feedback on if this is appropriate.

Thanks@HexDecimal. I just tested your implementation in Pydantic to implement anUNSET sentinel, andpickling seems to work as expected.

Edit: all of this is no longer relevant for this PR

Note on behavior regarding the sentinel registry and imported sentinels:

classMISSING:passassertSentinel("MISSING")isMISSING# Sentinel named after an existing non-Sentinel object

Which of these makes the most sense for the above code?

1. Register and return the existing class as the sentinel identity, the assert will pass (current PR implementation)
2. Return a new Sentinel identity, the assert will fail (old implementation)
3. Raise a warning, then do 1
4. RaiseTypeError becauseMISSING is not aSentinel (what I recommend, the most strict option)
5. Add astrict parameter toSentinel to decide between 1 and 4

Edit: currently going with 2 to simplify the implementation.

2nd behavior involving registered sentinel parameters:

MISSING=Sentinel("MISSING",repr="foo")assertMISSINGisSentinel("MISSING")# Implicit 'repr' conflicts with previous 'repr'assertMISSINGisSentinel("MISSING",repr="bar")# Explicit 'repr' conflicts with previous 'repr'

Which of these makes the most sense for the above code?

1. repr="foo" is locked in for all subsequent sentinels, subsequentrepr is always ignored, asserts pass (current implementation)
2. Both asserts raiseValueError as long asrepr="foo" is not given again (my recommendation, the most strict option)
3. repr only checked if explicitly given, 1st assert passes, 2nd assert raisesValueError
4. Checkrepr like 2 but raise warnings instead of errors
5. Addstrict parameter to decide between 1 and 2

Edit: currently going with 4 to not break anything.

The truthiness of sentinels is still being discussed. One thing which is unclear is if conversions to bool are common enough to be an issue in the first place (because sentinels are normally checked using identity comparison and little else). It'd be interesting to experiment with the most strict option and disable conversions to bool entirely:

def__bool__(self)->Never:raiseTypeError(...)

Which of these makes the most sense for the above code?

I feel strongly that 2 is the right behavior. Constructing a class shouldn't look around in the globals for other stuff that might be using the same name.

Constructing a class shouldn't look around in the globals for other stuff that might be using the same name.

That was necessary to handle unpickling until I finally implemented the correct reduce function, but at this point I can remove that code now and revert to the old behavior. I'd accept that the other options are unnecessarily handholdy.

Unless my removal of sentinel truthiness causes new issues then this PR is finished. I can rebase this if needed.

Nevermind, anything related to truthiness can be a separate PR. It's outside the scope of this one.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.38%. Comparing base (19cc3bc) to head (678b73b).

@@           Coverage Diff           @@##             main     #617   +/-   ##=======================================  Coverage   97.38%   97.38%           =======================================  Files           3        3             Lines        7689     7701   +12     =======================================+ Hits         7488     7500   +12  Misses        201      201

Flags with carried forward coverage won't be shown.Click here to find out more.

I've narrowed the scope of this PR down to only what's needed to support pickling sentinels. Unfortunately this still means thatrepr needs to be touched to allow for themodule_name parameter. Thankfully a lot of other changes were no longer needed after this latest rebase.

Has anyone mentioned that_marker is a terribly obfuscated name for a sentinel?