Repository: scrapy/scrapy
Pull request: Code cleanup scrapy.utils.python.WeakKeyCache #4684
Resolves #4684

Problem statement:
Should `scrapy.utils.python.WeakKeyCache` be deprecated?
## Summary
I can't find any reference to `scrapy.utils.python.WeakKeyCache` (except in tests) and I think that it should be deprecated.

## Motivation
Keeping non-used and non-documented code makes the codebase harder to maintain.

## Describe alternatives you've considered
As it's tested we could keep it, but I don't think it's a good idea.

Hints from issue discussion:
Looks like its (last) usage was removed in https://github.com/scrapy/scrapy/commit/332bf3b6b7bd238df5e8be601ee0686231a6776c
I can clean this up. Is it up for grabs?  Can someone assign this to me, please?
@Gallaecio @wRAR 

This would be my first open-source contribution :)
@kshitijcode No need to ask first, feel free to go ahead! :slightly_smiling_face: 
@Gallaecio @wRAR 
I have raised the PR with the necessary changes: https://github.com/scrapy/scrapy/pull/4701
But I am not sure what is failing the tests. 

Tests pass successfully locally after adding `-W ignore::pytest.PytestDeprecationWarning.` to tox.ini 
I am not really sure whats breaking in Azure Pipelines.
@kshitijcode the test failure is unrelated to your changes, I've created #4702 to track it

Commit message: deleted tester.py

Patch to review:
diff --git a/scrapy/utils/python.py b/scrapy/utils/python.py
index 9204977cf03..4756b07b622 100644
--- a/scrapy/utils/python.py
+++ b/scrapy/utils/python.py
@@ -7,10 +7,12 @@
 import re
 import sys
 import weakref
+import warnings
 from functools import partial, wraps
 from itertools import chain
 
 from scrapy.utils.decorators import deprecated
+from scrapy.exceptions import ScrapyDeprecationWarning
 
 
 def flatten(x):
@@ -127,6 +129,7 @@ def re_rsearch(pattern, text, chunk_size=1024):
     In case the pattern wasn't found, None is returned, otherwise it returns a tuple containing
     the start position of the match, and the ending (regarding the entire text).
     """
+
     def _chunk_iter():
         offset = len(text)
         while True:
@@ -158,6 +161,7 @@ def new_method(self, *args, **kwargs):
         if self not in cache:
             cache[self] = method(self, *args, **kwargs)
         return cache[self]
+
     return new_method
 
 
@@ -276,6 +280,7 @@ def equal_attributes(obj1, obj2, attributes):
 class WeakKeyCache:
 
     def __init__(self, default_factory):
+        warnings.warn("Call to deprecated Class WeakKeyCache", category=ScrapyDeprecationWarning, stacklevel=2)
         self.default_factory = default_factory
         self._weakdict = weakref.WeakKeyDictionary()
 


Code review comments:
diff_hunk: @@ -276,6 +280,7 @@ def equal_attributes(obj1, obj2, attributes):
 class WeakKeyCache:
 
     def __init__(self, default_factory):
+        warnings.warn("Call to deprecated Class WeakKeyCache", category=ScrapyDeprecationWarning, stacklevel=2)
path: scrapy/utils/python.py
text: @user1:
“Call” feels weird to me when talking about classes, and “class” should be lowercase.

:lipstick: What about…

```suggestion
        warnings.warn("The WeakKeyCache class is deprecated",
                      category=ScrapyDeprecationWarning, stacklevel=2)
```

@author:
@user1 resolved.

diff_hunk: @@ -7,10 +7,12 @@
 import re
 import sys
 import weakref
+import warnings
 from functools import partial, wraps
 from itertools import chain
 
 from scrapy.utils.decorators import deprecated
+from scrapy.exceptions import ScrapyDeprecationWarning
path: scrapy/utils/python.py
text: @user1:
:lipstick: 

```suggestion
from scrapy.exceptions import ScrapyDeprecationWarning
from scrapy.utils.decorators import deprecated
```