Skip to content

make function parameter sentinel value true singletons #88289

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
taleinat opened this issue May 13, 2021 · 13 comments
Open

make function parameter sentinel value true singletons #88289

taleinat opened this issue May 13, 2021 · 13 comments
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@taleinat
Copy link
Contributor

BPO 44123
Nosy @rhettinger, @vstinner, @taleinat, @ericvsmith, @zware, @serhiy-storchaka, @iritkatriel

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-05-13.13:17:06.374>
labels = ['type-feature', 'library', '3.11']
title = 'make function parameter sentinel value true singletons'
updated_at = <Date 2021-05-15.19:23:47.412>
user = 'https://github.com/taleinat'

bugs.python.org fields:

activity = <Date 2021-05-15.19:23:47.412>
actor = 'taleinat'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-05-13.13:17:06.374>
creator = 'taleinat'
dependencies = []
files = []
hgrepos = []
issue_num = 44123
keywords = []
message_count = 13.0
messages = ['393580', '393581', '393582', '393590', '393594', '393606', '393627', '393628', '393632', '393634', '393674', '393680', '393724']
nosy_count = 7.0
nosy_names = ['rhettinger', 'vstinner', 'taleinat', 'eric.smith', 'zach.ware', 'serhiy.storchaka', 'iritkatriel']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue44123'
versions = ['Python 3.11']

@taleinat
Copy link
Contributor Author

I recently noticed that some functions use sentinel values to differentiate between not having been passed any value vs. None. One example of this is traceback.print_exception(), hence I'm adding Irit to the nosy list.

However, using e.g. sentinel = object() or a single instance of a dedicated class/type can break when combined with pickling (see examples below).

Since these sentinel are usually compared using the is operator, having more than a single instance will break things, sometimes in very confusing ways.

I suggest ensuring that a single instance is always used, probably by using a class with a __new__() method making sure to always return a single instance.

>>> sentinel = object()
>>> sentinel2 = pickle.loads(pickle.dumps(sentinel))
>>> sentinel is sentinel2
False
>>> sentinel
<object object at 0x7fd00a986560>
>>> sentinel2
<object object at 0x7fd00a986500>


>>> class A:
...     pass
...     
>>> a = A()
>>> a2 = pickle.loads(pickle.dumps(a))
>>> a is a2
False
>>> a
<__main__.A object at 0x7fd00a9972f0>
>>> a2
<__main__.A object at 0x7fd009599450>

@taleinat taleinat added type-bug An unexpected behavior, bug, or error 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir labels May 13, 2021
@taleinat
Copy link
Contributor Author

Additional examples of such sentinel values in the stdlib are those in the dataclasses module, e.g. dataclasses.MISSING.

@zware
Copy link
Member

zware commented May 13, 2021

FWIW, at work we've been using ... as a handy not-None singleton.

@taleinat
Copy link
Contributor Author

Here is what I suggest working with sentinels would look like:

>>> from dataclasses import MISSING
>>> MISSING
dataclasses.MISSING
>>> M2 = pickle.loads(pickle.dumps(MISSING))
>>> M2
dataclasses.MISSING
>>> M2 is MISSING
True

Here's an implementation which ensures a single instance is used, even considering multi-threading and pickling, which sets a nice repr according to the module and class name:

try:
from threading import Lock
except ImportError:
class Lock:
def __enter__(self):
pass
def __exit__(self, exc_type, exc_value, traceback):
pass

class Sentinel:
    _instance = None
    _lock = Lock()
    def __new__(cls):
        if cls._instance is None:
            with cls._lock:
                if cls._instance is None:
                    cls._instance = super().__new__(cls)
        return cls._instance
    def __repr__(self):
        *path_parts, classname = self.__class__.__qualname__.split('.')
        return '.'.join([self.__class__.__module__, *path_parts, classname.removeprefix('_')])


class _MISSING(Sentinel):
    pass
MISSING = _MISSING()

@taleinat
Copy link
Contributor Author

Alternatively, sentinels can simply be classes:

class Sentinel:
    def __new__(cls, *args, **kwargs):
        raise TypeError(f'{cls.__qualname__} cannot be instantiated')

class MISSING(Sentinel):
    pass

@taleinat
Copy link
Contributor Author

... and they can be given excellent reprs by using a meta-class:

class Sentinel(type):
    @classmethod
    def __prepare__(cls, name, bases, **kwds):
        d = super().__prepare__(name, bases, **kwds)
        def __new__(cls_, *args, **kwargs):
            raise TypeError(
                f'{cls_!r} is a sentinel and cannot be instantiated')
        d.update(__new__=__new__)
        return d

    def __repr__(cls):
        return f'{cls.__module__}.{cls.__qualname__}'


class MISSING(metaclass=Sentinel): pass

This also has another nice benefit:

>>> type(MISSING)
<class 'sentinels.Sentinel'>

@rhettinger
Copy link
Contributor

This needs a good deal more discussion before sweeping through the code and change a long standing Python idiom that has stood the test of time. Until now, no one has claimed that this is broken. You "recently noticing this" doesn't mean that it is wrong.

@serhiy-storchaka
Copy link
Member

I do not understand the problem with pickling sentinel values used as default values for function parameters. Could you please show an example?

@taleinat
Copy link
Contributor Author

This needs a good deal more discussion before sweeping through the code and change a long standing Python idiom that has stood the test of time.

I agree this should be discussed, which is why I created this issue about it, as place for that discussion to take place. This is also being discussed on python-dev.

The examples I've mentioned are both rather new, and they have implemented sentinel values using three different methods, with those in dataclasses even being inconsistent among themselves. I fail to see how this can be considered "chang[ing] a long standing Python idiom that has stood the test of time".

You "recently noticing this" doesn't mean that it is wrong.

I find this wording makes things unnecessarily personal and negative. Let's discuss things based on technical merit.

@vstinner
Copy link
Member

Tal: "This is also being discussed on python-dev."

https://mail.python.org/archives/list/python-dev@python.org/thread/ZLVPD2OISI7M4POMTR2FCQTE6TPMPTO3/

@taleinat
Copy link
Contributor Author

I do not understand the problem with pickling sentinel values used as default values for function parameters. Could you please show an example?

A category of relevant uses I can think of is when wrapping a function and storing the parameters it was called with for some later use. Some examples of this are logging, caching and RPC.

Specifically for RPC, using pickle to serialize/deserialize the parameters and then call the function with them seems reasonable. RPyC[1] does this in some cases, though I haven't yet checked how it handles these kinds of sentinel objects specifically.

I'll try to find a good concrete example.

[1] https://rpyc.readthedocs.io/

@serhiy-storchaka
Copy link
Member

Parameters are not passed to a function. Arguments are passed. And what you need is just to not pass to the wrapped function arguments which were not passed to wrapper. *args and **kwargs do not contain arguments which were not passed.

I understand that it would be pleasant to have better some singletons and sentinels (with better repr, and sometimes pickleable), but I don't think that the problem with pickling default values of function parameters even exists.

@taleinat
Copy link
Contributor Author

And what you need is just to not pass to the wrapped function arguments which were not passed to wrapper. *args and **kwargs do not contain arguments which were not passed.

It's common for wrapper functions to mirror the defaults of the wrapped functions and pass those along... A quick search brings up an example from dataclasses where sentinel values are passed as arguments to a wrapped callable[1].

I concede that issues with pickling such values are very unlikely. I stumbled upon such an issue once and it was difficult to debug, but that was a very unusual circumstance.

Still, it's a bit surprising that we have public facing values in stdlib modules which don't behave as one would expect with regard to pickling. This is why I created this

It seems that I should have marked this as "enhancement" rather than the default of "behavior", though, since I meant this to be an improvement on the current situation, rather then assert the current situation is "broken".

[1]

def field(*, default=MISSING, default_factory=MISSING, init=True, repr=True,

@taleinat taleinat added type-feature A feature request or enhancement and removed 3.10 only security fixes type-bug An unexpected behavior, bug, or error labels May 15, 2021
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

5 participants