Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support for pickling sentinel objects as singletons #617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HexDecimal wants to merge 2 commits into python:main
base: main
Choose a base branch
Loading
from HexDecimal:conforming-sentinel

Conversation

@HexDecimal
Copy link
Contributor

@HexDecimal HexDecimal commented Jun 4, 2025
edited
Loading

My attempt at implementing PEP 661 since I was unhappy with #594. I'm hoping that this is a PR of decent quality and not just me desperately wanting pickle support.

Changes made to Sentinel:

  • Added module_name parameter following PEP implementation and tweaking that to use the local _caller helper function. Breaking change due to repr becoming a keyword parameter.
  • name requires qualified names to support pickle singletons, this is tested and documented.
  • Added __reduce__ to track Sentinel by name and module_name.
  • Added copy and pickle tests. There were several other use-cases for pickle from the PEP 661 discussion. The PEP requires that the sentinel identity is preserved during these operations.
  • Updated documentation for Sentinel.

For a while I still supported the repr parameter and after following the PEP 661 discussion I ended up implementing the truthiness tweaks mentioned there, but then I ended up scrapping all of that so that I could follow the PEP more closely, but they would be easy to reintroduce later if desired.

I'm unsure of how to mention version changes in the docs. Replacing repr with module_name is technically a breaking change.

Closes #720

Copy link

python-cla-bot bot commented Jun 4, 2025
edited
Loading

All commit authors signed the Contributor License Agreement.

CLA signed

Copy link
Contributor

Viicos commented Jun 5, 2025

Replacing repr with module_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:

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

Copy link
Contributor

Viicos commented Jun 5, 2025

Copy link
Member

I'm not willing to remove the repr parameter from typing_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.

Copy link
Contributor Author

I'm not willing to remove the repr parameter from typing_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 change repr into a keyword parameter.

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

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

@HexDecimal HexDecimal force-pushed the conforming-sentinel branch 2 times, most recently from 2c509de to e29210a Compare July 1, 2025 23:49
Copy link
Contributor Author

HexDecimal commented Jul 2, 2025
edited
Loading

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 raise pickle.PicklingError instead of TypeError. 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 a strict keyword parameter to Sentinel which prevents sentinels from being accidentally named after existing top-level objects which are not Sentinel as well as ensuring that repr is never given conflicting values. I'll need feedback on if this is appropriate. (削除ここまで)

Copy link
Contributor

Viicos commented Jul 2, 2025

Thanks @HexDecimal. I just tested your implementation in Pydantic to implement an UNSET sentinel, and pickling seems to work as expected.

HexDecimal reacted with thumbs up emoji

This comment was marked as outdated.

Copy link
Member

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.

Copy link
Contributor Author

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.

This comment was marked as outdated.

@HexDecimal HexDecimal marked this pull request as draft December 2, 2025 20:43
@HexDecimal HexDecimal changed the title (削除) Refactor Sentinel to conform to PEP 661 (削除ここまで) (追記) Support for pickling sentinel objects as singletons (追記ここまで) Dec 2, 2025
Copy link

codecov bot commented Dec 2, 2025
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.38%. Comparing base (a7610ef) to head (2238608).

@@ Coverage Diff @@
## main #617 +/- ##
=======================================
 Coverage 97.38% 97.38% 
=======================================
 Files 3 3 
 Lines 7689 7701 +12 
=======================================
+ Hits 7488 7500 +12 
 Misses 201 201 
Flag Coverage Δ
3.10 89.00% <100.00%> (+0.01%) ⬆️
3.10.4 89.00% <100.00%> (+0.01%) ⬆️
3.11 88.23% <100.00%> (+0.01%) ⬆️
3.11.0 87.46% <100.00%> (+0.01%) ⬆️
3.12 88.18% <100.00%> (+0.01%) ⬆️
3.12.0 88.16% <100.00%> (+0.01%) ⬆️
3.13 81.68% <100.00%> (+0.02%) ⬆️
3.13.0 82.41% <100.00%> (+0.02%) ⬆️
3.14 78.12% <100.00%> (+0.03%) ⬆️
3.9 89.71% <100.00%> (+0.01%) ⬆️
3.9.12 89.71% <100.00%> (+0.01%) ⬆️
pypy3.10 88.83% <100.00%> (+0.01%) ⬆️
pypy3.11 88.09% <100.00%> (+0.01%) ⬆️
pypy3.9 89.54% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
src/test_typing_extensions.py 98.39% <100.00%> (+<0.01%) ⬆️
src/typing_extensions.py 93.95% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor Author

I've narrowed the scope of this PR down to only what's needed to support pickling sentinels. Unfortunately this still means that repr needs to be touched to allow for the module_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?

def __init__(
self,
name: str,
module_name: typing.Optional[str] = None,
Copy link
Member

@JelleZijlstra JelleZijlstra Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new argument is problematic because it might (again) conflict with what PEP 661 proposes. I'd be OK with adding it but I'd make it keyword-only.

Copy link
Contributor Author

@HexDecimal HexDecimal Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make module_name keyword only, but what happens to repr? Do I revert repr back to a positional parameter?

Copy link
Contributor Author

@HexDecimal HexDecimal Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module_name is now keyword-only.

The `repr` parameters position was replaced by `module_name` to conform to PEP 661.
Added copy and pickle tests.
Updated documentation for Sentinel.
`_marker` was defined before `caller` which causes minor issues,
resolved by setting its module name manually.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

@Viicos Viicos Awaiting requested review from Viicos

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing pickle support for Sentinel

AltStyle によって変換されたページ (->オリジナル) /