Activity

1. scoder

   added

   type-featureA feature request or enhancementA feature request or enhancement

   on Oct 24, 2023
2. AlexWaygood

   added

   topic-C-API

   on Oct 24, 2023
3. serhiy-storchaka commented on Oct 24, 2023

   

   serhiy-storchaka

   on Oct 24, 2023

   Member

   More actions

   We should first discuss the design.

   The flaws of the current _PyDict_Pop():

   • It is difficult to distinguish two cases: return value removed from the dict and returning the increfed default value. If the dict can contain arbitrary values, you need to create a special sentinel value.
   • If you do not need the default value, you still have to decref it.

   Alternative designs of PyDict_Pop() that do not raise KeyError.

   1. PyObject *PyDict_Pop(PyObject *dict, PyObject *key)

      • Return NULL and set exception on error.
      • Return NULL and not set exception if the key is not found.
      • Return the removed value if the key is found.

      You have to use PyErr_Occurred() to distinguish between first two cases.

   2. int PyDict_Pop(PyObject *dict, PyObject *key, PyObject **result)

      • Return -1, set *result to NULL and set exception on error.
      • Return 0, set *result to NULL and not set exception if the key is not found.
      • Return 1, set *result to the removed value if the key is found.

      You have not to use PyErr_Occurred(). You have to check the returning code or *result and *result to the default values after the call if you need it.

   3. int PyDict_Pop(PyObject *dict, PyObject *key, PyObject **result)

      • Return -1, set *result to NULL and set exception on error.
      • Return 0, incref *result (which should be initialized with the default value) and not set exception if the key is not found.
      • Return 1, set *result to the removed value if the key is found.

      You have to always initialize *result before the call, but you can simply use *result after the call if you treat two last cases the same.

   These variants do not raise KeyError. If you need KeyError, you have to raise it yourself. Or we can introduce new function which pops item and raises KeyError, or add a flag to raise KeyError. The last two variants have subvariants, how to treat result == NULL:

   • Simply drop the removed value. It is convenient if PyDict_Pop() is used as a variant of PyDict_DelItem() that tolerates absent keys. It is the most of use cases in the CPython sources.
   • Treat it as a signal to raise KeyError.

   cc @rhettinger, @vstinner
4. vstinner commented on Oct 24, 2023

   

   vstinner

   on Oct 24, 2023

   Member

   More actions

   I like to return (-1, 0, 1). But I didn't get details of the API. The definition has not default argument? What is the behavior is default is NULL and the key does not exist.

   Is it possible to call it with key=NULL?
5. scoder commented on Oct 25, 2023

   

   scoder

   on Oct 25, 2023

   ContributorAuthor

   More actions

   Ok, I get the pattern. I'll add _PyDict_Pop() to #111162 so that we have an API until the bike-shedding has settled.

   Reacted by Serhiy Storchaka
6. scoder

   added a commit that references this issue on Oct 25, 2023

   pythongh-111262: Re-add _PyDict_Pop() to have a C-API until a new pub…

   794eb8a
7. encukou

   added a commit that references this issue on Oct 25, 2023

   gh-106320: Re-add some PyLong/PyDict C-API functions (GH-#111162)

   a8a89fc
8. vstinner commented on Oct 27, 2023

   

   vstinner

   on Oct 27, 2023

   Member

   More actions

   Ok, I get the pattern. I'll add _PyDict_Pop() to #111162 so that we have an API until the bike-shedding has settled.

   That sounds reasonable since 300+ private functions were removed, I don't expect that we can add public replacement quickly enough.
9. vstinner

   added 13 commits that reference this issue on Nov 12, 2023

   pythongh-111262: Add PyDict_Pop() function

   b3e7de4

   pythongh-111262: Add PyDict_Pop() function

   d2add05

   pythongh-111262: Add PyDict_Pop() function

   dd1ce79

   pythongh-111262: Add PyDict_Pop() function

   8c3ab39

   pythongh-111262: Add PyDict_Pop() function

   2f5f93a

   pythongh-111262: Add PyDict_Pop() function

   ea2aab1

   pythongh-111262: Add PyDict_Pop() function

   bb8358a

   pythongh-111262: Add PyDict_Pop() function

   148d06c

   pythongh-111262: Add PyDict_Pop() function

   e2614b6

   pythongh-111262: Add PyDict_Pop() function

   cc0ad9d

   pythongh-111262: Add PyDict_Pop() function

   0693e77

   pythongh-111262: Add PyDict_Pop() function

   1498269

   gh-111262: Add PyDict_Pop() function (#112028)

   4f04172
10. vstinner commented on Nov 14, 2023

    

    vstinner

    on Nov 14, 2023

    Member

    More actions

    After a long discussion, the int PyDict_Pop(PyObject *p, PyObject *key, PyObject **result) API won: there is no default value anymore. Thanks everyone for this interesting discussion.

    Thanks @scoder for raising the need for such API.
11. vstinner

    closed this as completedon Nov 14, 2023
12. vstinner commented on Nov 14, 2023

    

    vstinner

    on Nov 14, 2023

    Member

    More actions

    I wrote python/pythoncapi-compat#81 to add PyDict_Pop() and PyDict_PopString() to pythoncapi-compat: it calls _PyDict_Pop() on CPython >= 3.5.0b2, or call the dict.pop() method on older Cython versions and on PyPy.
13. aisk

    added 2 commits that reference this issue on Feb 11, 2024

    pythongh-106320: Re-add some PyLong/PyDict C-API functions (GH-python…

    0956570

    pythongh-111262: Add PyDict_Pop() function (python#112028)

    024183c
14. Glyphack

    added 2 commits that reference this issue on Sep 2, 2024

    pythongh-106320: Re-add some PyLong/PyDict C-API functions (GH-python…

    4aa7b76

    pythongh-111262: Add PyDict_Pop() function (python#112028)

    99d4048