From 8b08ed5249ae08442dacb18f0b50530b4c8bb6d7 Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Wed, 9 Oct 2024 18:07:24 +0300 Subject: [PATCH 01/14] impl --- Lib/functools.py | 4 ++++ Lib/test/test_functools.py | 6 ++++++ Modules/_functoolsmodule.c | 33 ++++++++++++++++++--------------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index 9d53d3601559b2..c42f608c38be6c 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -326,6 +326,10 @@ def _partial_new(cls, func, /, *args, **keywords): "or a descriptor") if args and args[-1] is Placeholder: raise TypeError("trailing Placeholders are not allowed") + if keywords: + for v in keywords.values(): + if v is Placeholder: + raise TypeError("keyword Placeholders are not allowed") if isinstance(func, base_cls): pto_phcount = func._phcount tot_args = func.args diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index bdaa9a7ec4f020..5d937424218c2a 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -249,6 +249,12 @@ def test_placeholders_optimization(self): self.assertEqual(p2.args, (PH, 0)) self.assertEqual(p2(1), ((1, 0), {})) + def test_placeholders_kw_restriction(self): + PH = self.module.Placeholder + exc_string = 'keyword Placeholders are not allowed' + with self.assertRaisesRegex(TypeError, exc_string): + self.partial(capture, a=PH) + def test_construct_placeholder_singleton(self): PH = self.module.Placeholder tp = type(PH) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 31cf7bcc09782c..ceaa3fa5e24792 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -277,30 +277,33 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) assert(PyTuple_Check(pto->args)); } + /* process keywords */ if (pto_kw == NULL || PyDict_GET_SIZE(pto_kw) == 0) { - if (kw == NULL) { - pto->kw = PyDict_New(); - } - else if (Py_REFCNT(kw) == 1) { - pto->kw = Py_NewRef(kw); - } - else { - pto->kw = PyDict_Copy(kw); - } + pto->kw = PyDict_New(); } else { pto->kw = PyDict_Copy(pto_kw); - if (kw != NULL && pto->kw != NULL) { - if (PyDict_Merge(pto->kw, kw, 1) != 0) { - Py_DECREF(pto); - return NULL; - } - } } if (pto->kw == NULL) { Py_DECREF(pto); return NULL; } + if (kw != NULL) { + PyObject *key, *val; + Py_ssize_t pos = 0; + while (PyDict_Next(kw, &pos, &key, &val)) { + if (val == phold) { + Py_DECREF(pto); + PyErr_SetString(PyExc_TypeError, + "keyword Placeholders are not allowed"); + return NULL; + } + if (PyDict_SetItem(pto->kw, key, val)) { + Py_DECREF(pto); + return NULL; + } + } + } partial_setvectorcall(pto); return (PyObject *)pto; From bea722a6b7a76d73d9fcff5ea424c710570894f6 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:16:11 +0000 Subject: [PATCH 02/14] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst diff --git a/Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst b/Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst new file mode 100644 index 00000000000000..8320d6b6c10a1d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst @@ -0,0 +1 @@ +:data:`functools.Placeholder` is not allowed in :func:`functools.partial` keyword vaules. From e741a5ca4f54e23cfb8ead092b2b9f5ca8a47106 Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Mon, 28 Oct 2024 08:51:17 +0200 Subject: [PATCH 03/14] remove news --- .../next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst diff --git a/Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst b/Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst deleted file mode 100644 index 8320d6b6c10a1d..00000000000000 --- a/Misc/NEWS.d/next/Library/2024-10-09-15-16-09.gh-issue-125028.IOzxPL.rst +++ /dev/null @@ -1 +0,0 @@ -:data:`functools.Placeholder` is not allowed in :func:`functools.partial` keyword vaules. From 71e65c300b7fec2e09681ca3ce16e8d0ce9b7361 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 06:54:23 +0000 Subject: [PATCH 04/14] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst diff --git a/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst b/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst new file mode 100644 index 00000000000000..c988fe27b79a8f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst @@ -0,0 +1 @@ +:data:`functools.Placeholder` is restricted for :func:`functools.partial` keyword vaules. From 294f189e7885fa2fb16843a93507ae588bf1a815 Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Mon, 28 Oct 2024 09:46:17 +0200 Subject: [PATCH 05/14] no loop for check --- Lib/functools.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index c42f608c38be6c..44de03e413a578 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -326,10 +326,8 @@ def _partial_new(cls, func, /, *args, **keywords): "or a descriptor") if args and args[-1] is Placeholder: raise TypeError("trailing Placeholders are not allowed") - if keywords: - for v in keywords.values(): - if v is Placeholder: - raise TypeError("keyword Placeholders are not allowed") + if keywords and Placeholder in keywords.values(): + raise TypeError("keyword Placeholders are not allowed") if isinstance(func, base_cls): pto_phcount = func._phcount tot_args = func.args From 5abc61e16783088fd721fd054e604205dcf8914d Mon Sep 17 00:00:00 2001 From: dgpb <3577712+dg-pb@users.noreply.github.com> Date: Sun, 5 Jan 2025 10:03:21 +0200 Subject: [PATCH 06/14] Idiomatic C Co-authored-by: Erlend E. Aasland --- Modules/_functoolsmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index dc5e4229670797..b0f1bddc96f212 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -301,7 +301,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) "keyword Placeholders are not allowed"); return NULL; } - if (PyDict_SetItem(pto->kw, key, val)) { + if (PyDict_SetItem(pto->kw, key, val) < 0) { Py_DECREF(pto); return NULL; } From 656361b4f5adb64ed221cd0903fe8867a1387c15 Mon Sep 17 00:00:00 2001 From: dgpb <3577712+dg-pb@users.noreply.github.com> Date: Sun, 5 Jan 2025 10:03:43 +0200 Subject: [PATCH 07/14] remove unrelated changes Co-authored-by: Erlend E. Aasland --- Modules/_functoolsmodule.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index b0f1bddc96f212..8629e4d247c781 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -280,7 +280,6 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) assert(PyTuple_Check(pto->args)); } - /* process keywords */ if (pto_kw == NULL || PyDict_GET_SIZE(pto_kw) == 0) { pto->kw = PyDict_New(); } From da4214a741f92c0eba28f0bd6c89e4ca5e8c0d66 Mon Sep 17 00:00:00 2001 From: dgpb <3577712+dg-pb@users.noreply.github.com> Date: Sun, 5 Jan 2025 14:56:32 +0200 Subject: [PATCH 08/14] loosen the regex in test Co-authored-by: Erlend E. Aasland --- Lib/test/test_functools.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 041a8fa92490f7..e7d89cb384f0e2 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -251,8 +251,7 @@ def test_placeholders_optimization(self): def test_placeholders_kw_restriction(self): PH = self.module.Placeholder - exc_string = 'keyword Placeholders are not allowed' - with self.assertRaisesRegex(TypeError, exc_string): + with self.assertRaisesRegex(TypeError, "Placeholder"): self.partial(capture, a=PH) def test_construct_placeholder_singleton(self): From 26dded844d7731da39e7f1c49ec524fe1225b7b0 Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Sun, 5 Jan 2025 15:02:33 +0200 Subject: [PATCH 09/14] improved error message --- Lib/functools.py | 2 +- Modules/_functoolsmodule.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index 44de03e413a578..af934801fb9d5d 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -327,7 +327,7 @@ def _partial_new(cls, func, /, *args, **keywords): if args and args[-1] is Placeholder: raise TypeError("trailing Placeholders are not allowed") if keywords and Placeholder in keywords.values(): - raise TypeError("keyword Placeholders are not allowed") + raise TypeError("Placeholder cannot be passed as a keyword argument") if isinstance(func, base_cls): pto_phcount = func._phcount tot_args = func.args diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 8629e4d247c781..a12f3ec163a050 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -297,7 +297,7 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) if (val == phold) { Py_DECREF(pto); PyErr_SetString(PyExc_TypeError, - "keyword Placeholders are not allowed"); + "Placeholder cannot be passed as a keyword argument"); return NULL; } if (PyDict_SetItem(pto->kw, key, val) < 0) { From e8d7521f2506b3dce87aae6651810b7fd3552b0e Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Sun, 5 Jan 2025 23:39:01 +0200 Subject: [PATCH 10/14] doc updates --- Doc/library/functools.rst | 3 +-- .../Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Doc/library/functools.rst b/Doc/library/functools.rst index e26a2226aa947a..93493614dac187 100644 --- a/Doc/library/functools.rst +++ b/Doc/library/functools.rst @@ -403,8 +403,7 @@ The :mod:`functools` module defines the following functions: >>> remove_first_dear(message) 'Hello, dear world!' - :data:`!Placeholder` has no special treatment when used in a keyword - argument to :func:`!partial`. + :data:`!Placeholder` cannot be passed to :func:`!partial` as a keyword argument. .. versionchanged:: 3.14 Added support for :data:`Placeholder` in positional arguments. diff --git a/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst b/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst index c988fe27b79a8f..09ebee4d41b68e 100644 --- a/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst +++ b/Misc/NEWS.d/next/Library/2024-10-28-06-54-22.gh-issue-125028.GEY8Ws.rst @@ -1 +1 @@ -:data:`functools.Placeholder` is restricted for :func:`functools.partial` keyword vaules. +:data:`functools.Placeholder` cannot be passed to :func:`functools.partial` as a keyword argument. From 106e6bc10b664b377b9aefb3049ae72d6f70c765 Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Mon, 6 Jan 2025 14:57:14 +0200 Subject: [PATCH 11/14] decouple error checking and kw merging + tests --- Lib/functools.py | 5 +++-- Lib/test/test_functools.py | 11 ++++++++++ Modules/_functoolsmodule.c | 45 ++++++++++++++++++++++++-------------- 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index af934801fb9d5d..d9550266771f51 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -326,8 +326,9 @@ def _partial_new(cls, func, /, *args, **keywords): "or a descriptor") if args and args[-1] is Placeholder: raise TypeError("trailing Placeholders are not allowed") - if keywords and Placeholder in keywords.values(): - raise TypeError("Placeholder cannot be passed as a keyword argument") + for value in keywords.values(): + if value is Placeholder: + raise TypeError("Placeholder cannot be passed as a keyword argument") if isinstance(func, base_cls): pto_phcount = func._phcount tot_args = func.args diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index e7d89cb384f0e2..db916becc42a91 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -233,6 +233,12 @@ def test_placeholders(self): actual_args, actual_kwds = p('x', 'y') self.assertEqual(actual_args, ('x', 0, 'y', 1)) self.assertEqual(actual_kwds, {}) + # Checks via `is` and not `eq` + # thus unittest.mock.ANY isn't treated as Placeholder + p = self.partial(capture, unittest.mock.ANY) + actual_args, actual_kwds = p() + self.assertEqual(actual_args, (unittest.mock.ANY,)) + self.assertEqual(actual_kwds, {}) def test_placeholders_optimization(self): PH = self.module.Placeholder @@ -253,6 +259,11 @@ def test_placeholders_kw_restriction(self): PH = self.module.Placeholder with self.assertRaisesRegex(TypeError, "Placeholder"): self.partial(capture, a=PH) + # Passes, as checks via `is` and not `eq` + p = self.partial(capture, a=unittest.mock.ANY) + actual_args, actual_kwds = p() + self.assertEqual(actual_args, ()) + self.assertEqual(actual_kwds, {'a': unittest.mock.ANY}) def test_construct_placeholder_singleton(self): PH = self.module.Placeholder diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index a12f3ec163a050..2db71fd864b006 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -196,6 +196,19 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) return NULL; } + /* keyword Placeholder prohibition */ + if (kw != NULL) { + PyObject *key, *val; + Py_ssize_t pos = 0; + while (PyDict_Next(kw, &pos, &key, &val)) { + if (val == phold) { + PyErr_SetString(PyExc_TypeError, + "Placeholder cannot be passed as a keyword argument"); + return NULL; + } + } + } + /* check wrapped function / object */ pto_args = pto_kw = NULL; int res = PyObject_TypeCheck(func, state->partial_type); @@ -281,31 +294,29 @@ partial_new(PyTypeObject *type, PyObject *args, PyObject *kw) } if (pto_kw == NULL || PyDict_GET_SIZE(pto_kw) == 0) { - pto->kw = PyDict_New(); + if (kw == NULL) { + pto->kw = PyDict_New(); + } + else if (Py_REFCNT(kw) == 1) { + pto->kw = Py_NewRef(kw); + } + else { + pto->kw = PyDict_Copy(kw); + } } else { pto->kw = PyDict_Copy(pto_kw); - } - if (pto->kw == NULL) { - Py_DECREF(pto); - return NULL; - } - if (kw != NULL) { - PyObject *key, *val; - Py_ssize_t pos = 0; - while (PyDict_Next(kw, &pos, &key, &val)) { - if (val == phold) { - Py_DECREF(pto); - PyErr_SetString(PyExc_TypeError, - "Placeholder cannot be passed as a keyword argument"); - return NULL; - } - if (PyDict_SetItem(pto->kw, key, val) < 0) { + if (kw != NULL && pto->kw != NULL) { + if (PyDict_Merge(pto->kw, kw, 1) != 0) { Py_DECREF(pto); return NULL; } } } + if (pto->kw == NULL) { + Py_DECREF(pto); + return NULL; + } partial_setvectorcall(pto); return (PyObject *)pto; From 5e151aaba749f62b080f063dc175d404dcf5c33b Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Tue, 7 Jan 2025 01:11:22 +0200 Subject: [PATCH 12/14] make test meaningful for ANY check --- Lib/test/test_functools.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index db916becc42a91..bef95d9c9fb31a 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -237,7 +237,8 @@ def test_placeholders(self): # thus unittest.mock.ANY isn't treated as Placeholder p = self.partial(capture, unittest.mock.ANY) actual_args, actual_kwds = p() - self.assertEqual(actual_args, (unittest.mock.ANY,)) + self.assertEqual(len(actual_args), 1) + self.assertIs(actual_args[0], unittest.mock.ANY) self.assertEqual(actual_kwds, {}) def test_placeholders_optimization(self): From 38ff4718d094c035cbbfaefd3329604958e2931c Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Tue, 7 Jan 2025 14:14:37 +0200 Subject: [PATCH 13/14] Use ALWAYS_EQ instead of ANY --- Lib/test/test_functools.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index bef95d9c9fb31a..271778df268adc 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -20,6 +20,7 @@ import contextlib from inspect import Signature +from test.support import ALWAYS_EQ from test.support import import_helper from test.support import threading_helper @@ -235,10 +236,10 @@ def test_placeholders(self): self.assertEqual(actual_kwds, {}) # Checks via `is` and not `eq` # thus unittest.mock.ANY isn't treated as Placeholder - p = self.partial(capture, unittest.mock.ANY) + p = self.partial(capture, ALWAYS_EQ) actual_args, actual_kwds = p() self.assertEqual(len(actual_args), 1) - self.assertIs(actual_args[0], unittest.mock.ANY) + self.assertIs(actual_args[0], ALWAYS_EQ) self.assertEqual(actual_kwds, {}) def test_placeholders_optimization(self): @@ -261,10 +262,11 @@ def test_placeholders_kw_restriction(self): with self.assertRaisesRegex(TypeError, "Placeholder"): self.partial(capture, a=PH) # Passes, as checks via `is` and not `eq` - p = self.partial(capture, a=unittest.mock.ANY) + p = self.partial(capture, a=ALWAYS_EQ) actual_args, actual_kwds = p() self.assertEqual(actual_args, ()) - self.assertEqual(actual_kwds, {'a': unittest.mock.ANY}) + self.assertEqual(len(actual_kwds), 1) + self.assertIs(actual_kwds['a'], ALWAYS_EQ) def test_construct_placeholder_singleton(self): PH = self.module.Placeholder From 85955d691c55da8428d3a8473512c3af03d8524f Mon Sep 17 00:00:00 2001 From: "d.grigonis" Date: Tue, 7 Jan 2025 16:12:01 +0200 Subject: [PATCH 14/14] comment update --- Lib/test/test_functools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 271778df268adc..eb5ee31fd16a3f 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -235,7 +235,7 @@ def test_placeholders(self): self.assertEqual(actual_args, ('x', 0, 'y', 1)) self.assertEqual(actual_kwds, {}) # Checks via `is` and not `eq` - # thus unittest.mock.ANY isn't treated as Placeholder + # thus ALWAYS_EQ isn't treated as Placeholder p = self.partial(capture, ALWAYS_EQ) actual_args, actual_kwds = p() self.assertEqual(len(actual_args), 1)