From: Nils Philippsen <nils@tiptoe.de>
Date: Tue, 28 Feb 2023 21:04:54 +0000 (-0500)
Subject: restore old *args approach for MutableDict.pop()
X-Git-Tag: rel_2_0_5~6^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=80b7c608835f1be5418d8ed567664654e8b583e3;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

restore old *args approach for MutableDict.pop()

The typing change in ba0e508141206efc55cdab91df21c1
changed the semantics of pop() and possibly setdefault() in order to
try working at runtime with a two-argument signature.  however
the implementation for this in cpython likely uses a strict `*args`
approach where the lack of the second parameter is explicit, rather
than matching to a constant.  Restore the old implementation inside
of a "not TYPE_CHECKING" block while keeping the type annotated forms
intact for typing only.

Fixed regression caused by typing added to ``sqlalchemy.ext.mutable`` for
:ticket:`8667`, where the semantics of the ``.pop()`` method changed such
that the method was non-working.  Pull request courtesy Nils Philippsen.

Fixes: #9380
Closes: #9381
Co-authored-by: Mike Bayer <mike_mp@zzzcomputing.com>
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/9381
Pull-request-sha: fd903ce1b949d2af26ceb6c2159ad84aab007f3d

Change-Id: I213e52f51a795801aacf05307e38cc8c89b54e12
---

diff --git a/doc/build/changelog/unreleased_20/9380.rst b/doc/build/changelog/unreleased_20/9380.rst
new file mode 100644
index 0000000000..6ac454e27c
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/9380.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, ext, regression
+    :tickets: 9380
+
+    Fixed regression caused by typing added to ``sqlalchemy.ext.mutable`` for
+    :ticket:`8667`, where the semantics of the ``.pop()`` method changed such
+    that the method was non-working. Pull request courtesy Nils Philippsen.
diff --git a/lib/sqlalchemy/ext/mutable.py b/lib/sqlalchemy/ext/mutable.py
index 07f357bd6b..940d62b0aa 100644
--- a/lib/sqlalchemy/ext/mutable.py
+++ b/lib/sqlalchemy/ext/mutable.py
@@ -369,6 +369,7 @@ from typing import Optional
 from typing import overload
 from typing import Set
 from typing import Tuple
+from typing import TYPE_CHECKING
 from typing import TypeVar
 from typing import Union
 import weakref
@@ -781,27 +782,25 @@ class MutableDict(Mutable, Dict[_KT, _VT]):
         super().__setitem__(key, value)
         self.changed()
 
-    def _exists(self, value: _T | None) -> TypeGuard[_T]:
-        return value is not None
+    if TYPE_CHECKING:
 
-    def _is_none(self, value: _T | None) -> TypeGuard[None]:
-        return value is None
+        @overload
+        def setdefault(self, key: _KT) -> _VT | None:
+            ...
 
-    @overload
-    def setdefault(self, key: _KT) -> _VT | None:
-        ...
+        @overload
+        def setdefault(self, key: _KT, value: _VT) -> _VT:
+            ...
 
-    @overload
-    def setdefault(self, key: _KT, value: _VT) -> _VT:
-        ...
+        def setdefault(self, key: _KT, value: _VT | None = None) -> _VT | None:
+            ...
 
-    def setdefault(self, key: _KT, value: _VT | None = None) -> _VT | None:
-        if self._exists(value):
-            result = super().setdefault(key, value)
-        else:
-            result = super().setdefault(key)  # type: ignore[call-arg]
-        self.changed()
-        return result
+    else:
+
+        def setdefault(self, *arg):  # noqa: F811
+            result = super().setdefault(*arg)
+            self.changed()
+            return result
 
     def __delitem__(self, key: _KT) -> None:
         """Detect dictionary del events and emit change events."""
@@ -812,21 +811,27 @@ class MutableDict(Mutable, Dict[_KT, _VT]):
         super().update(*a, **kw)
         self.changed()
 
-    @overload
-    def pop(self, __key: _KT) -> _VT:
-        ...
+    if TYPE_CHECKING:
 
-    @overload
-    def pop(self, __key: _KT, __default: _VT | _T) -> _VT | _T:
-        ...
+        @overload
+        def pop(self, __key: _KT) -> _VT:
+            ...
 
-    def pop(self, __key: _KT, __default: _VT | _T | None = None) -> _VT | _T:
-        if self._exists(__default):
-            result = super().pop(__key, __default)
-        else:
-            result = super().pop(__key)
-        self.changed()
-        return result
+        @overload
+        def pop(self, __key: _KT, __default: _VT | _T) -> _VT | _T:
+            ...
+
+        def pop(
+            self, __key: _KT, __default: _VT | _T | None = None
+        ) -> _VT | _T:
+            ...
+
+    else:
+
+        def pop(self, *arg):  # noqa: F811
+            result = super().pop(*arg)
+            self.changed()
+            return result
 
     def popitem(self) -> Tuple[_KT, _VT]:
         result = super().popitem()
diff --git a/test/ext/test_mutable.py b/test/ext/test_mutable.py
index 50dc22e351..290518dd67 100644
--- a/test/ext/test_mutable.py
+++ b/test/ext/test_mutable.py
@@ -349,6 +349,19 @@ class _MutableDictTestBase(_MutableDictTestFixture):
 
         eq_(f1.data, {"c": "d"})
 
+    def test_pop_default_none(self):
+        sess = fixture_session()
+
+        f1 = Foo(data={"a": "b", "c": "d"})
+        sess.add(f1)
+        sess.commit()
+
+        eq_(f1.data.pop("a", None), "b")
+        eq_(f1.data.pop("a", None), None)
+        sess.commit()
+
+        eq_(f1.data, {"c": "d"})
+
     def test_popitem(self):
         sess = fixture_session()
 
@@ -386,6 +399,10 @@ class _MutableDictTestBase(_MutableDictTestFixture):
 
         eq_(f1.data, {"a": "b", "c": "d"})
 
+        eq_(f1.data.setdefault("w", None), None)
+        sess.commit()
+        eq_(f1.data, {"a": "b", "c": "d", "w": None})
+
     def test_replace(self):
         sess = fixture_session()
         f1 = Foo(data={"a": "b"})