From 080b9bd0b8f65dcf09ea8ad94505fec6c77f820c Mon Sep 17 00:00:00 2001
From: Laerte Pereira <laertefbk@gmail.com>
Date: Thu, 22 Jun 2023 23:58:03 -0300
Subject: [PATCH 1/3] chore: Implement `pop` method on `BaseSettings` class

---
 scrapy/settings/__init__.py     | 14 ++++++++++++++
 tests/test_settings/__init__.py | 13 +++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py
index a3b849f7b..57fe1d17a 100644
--- a/scrapy/settings/__init__.py
+++ b/scrapy/settings/__init__.py
@@ -75,6 +75,8 @@ class BaseSettings(MutableMapping):
     highest priority will be retrieved.
     """
 
+    __default = object()
+
     def __init__(self, values=None, priority="project"):
         self.frozen = False
         self.attributes = {}
@@ -445,6 +447,18 @@ class BaseSettings(MutableMapping):
         else:
             p.text(pformat(self.copy_to_dict()))
 
+    def pop(self, name, default=__default):
+        try:
+            value = self.attributes[name]
+        except KeyError:
+            if default is self.__default:
+                raise
+
+            return SettingsAttribute(default, get_settings_priority("project"))
+        else:
+            del self.attributes[name]
+            return value
+
 
 class Settings(BaseSettings):
     """
diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py
index 4a577cd8c..0e2f4aa98 100644
--- a/tests/test_settings/__init__.py
+++ b/tests/test_settings/__init__.py
@@ -451,6 +451,19 @@ class SettingsTest(unittest.TestCase):
         self.assertIsInstance(myhandler_instance, FileDownloadHandler)
         self.assertTrue(hasattr(myhandler_instance, "download_request"))
 
+    def test_pop_item_with_default_value(self):
+        settings = Settings()
+
+        with self.assertRaises(KeyError):
+            settings.pop("DUMMY_CONFIG")
+
+        dummy_config = settings.pop("DUMMY_CONFIG", "dummy_value")
+
+        self.assertEqual(
+            repr(dummy_config), "<SettingsAttribute value='dummy_value' priority=20>"
+        )
+        self.assertEqual(dummy_config.value, "dummy_value")
+
 
 if __name__ == "__main__":
     unittest.main()

From 876feaf339e181c9c5a6b9a5f8ffedc03a9ed3d2 Mon Sep 17 00:00:00 2001
From: Laerte Pereira <laertefbk@gmail.com>
Date: Fri, 23 Jun 2023 00:14:31 -0300
Subject: [PATCH 2/3] chore: Use dunder to delete item instead of del keyword
 to handle immutable settings

---
 scrapy/settings/__init__.py     |  2 +-
 tests/test_settings/__init__.py | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/scrapy/settings/__init__.py b/scrapy/settings/__init__.py
index 57fe1d17a..8b3bdbabe 100644
--- a/scrapy/settings/__init__.py
+++ b/scrapy/settings/__init__.py
@@ -456,7 +456,7 @@ class BaseSettings(MutableMapping):
 
             return SettingsAttribute(default, get_settings_priority("project"))
         else:
-            del self.attributes[name]
+            self.__delitem__(name)
             return value
 
 
diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py
index 0e2f4aa98..125b1d96f 100644
--- a/tests/test_settings/__init__.py
+++ b/tests/test_settings/__init__.py
@@ -464,6 +464,22 @@ class SettingsTest(unittest.TestCase):
         )
         self.assertEqual(dummy_config.value, "dummy_value")
 
+    def test_pop_item_with_frozen_settings(self):
+        settings = Settings(
+            {"DUMMY_CONFIG": "dummy_value", "OTHER_DUMMY_CONFIG": "other_dummy_value"}
+        )
+
+        self.assertEqual(settings.pop("DUMMY_CONFIG").value, "dummy_value")
+
+        settings.freeze()
+
+        with self.assertRaises(TypeError) as error:
+            settings.pop("OTHER_DUMMY_CONFIG")
+
+        self.assertEqual(
+            str(error.exception), "Trying to modify an immutable Settings object"
+        )
+
 
 if __name__ == "__main__":
     unittest.main()

From a3f8912d69eacdd2208617e6afb418e4e1847e36 Mon Sep 17 00:00:00 2001
From: Laerte Pereira <laertefbk@gmail.com>
Date: Fri, 23 Jun 2023 00:15:32 -0300
Subject: [PATCH 3/3] chore: Rename test

---
 tests/test_settings/__init__.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_settings/__init__.py b/tests/test_settings/__init__.py
index 125b1d96f..bb6dc67fa 100644
--- a/tests/test_settings/__init__.py
+++ b/tests/test_settings/__init__.py
@@ -464,7 +464,7 @@ class SettingsTest(unittest.TestCase):
         )
         self.assertEqual(dummy_config.value, "dummy_value")
 
-    def test_pop_item_with_frozen_settings(self):
+    def test_pop_item_with_immutable_settings(self):
         settings = Settings(
             {"DUMMY_CONFIG": "dummy_value", "OTHER_DUMMY_CONFIG": "other_dummy_value"}
         )