Repository: scrapy/scrapy
Pull request #3899 (2019-07-24 06:54:04)

[Py3] Item loaders: allow single argument functions as processors
Python 3 only: `__qualname__` was added in Python 3.3 ([PEP 3155](https://www.python.org/dev/peps/pep-3155/)). I thought about a py2-compatible version using `six.get_method_function` (which accesses `__func__` in py3 and `im_func` in py2) and doing some regex checks on the obtained function's name, but that wasn't pretty. Since we're dropping py2 support soon and this is technically backward incompatible I figured it would be ok to leave this for Scrapy 2.0

Initially reported in #2735 

codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/3899?src=pr&el=h1) Report
> Merging [#3899](https://codecov.io/gh/scrapy/scrapy/pull/3899?src=pr&el=desc) into [master](https://codecov.io/gh/scrapy/scrapy/commit/9c514b976ffdf069b81c4b7728a7e8e531710680?src=pr&el=desc) will **decrease** coverage by `3.04%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master    #3899      +/-   ##
==========================================
- Coverage   85.47%   82.42%   -3.05%     
==========================================
  Files         165      165              
  Lines        9624     9629       +5     
  Branches     1446     1447       +1     
==========================================
- Hits         8226     7937     -289     
- Misses       1145     1423     +278     
- Partials      253      269      +16
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/3899?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/loader/\_\_init\_\_.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xvYWRlci9fX2luaXRfXy5weQ==) | `95.51% <100%> (+0.14%)` | :arrow_up: |
| [scrapy/linkextractors/sgml.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmtleHRyYWN0b3JzL3NnbWwucHk=) | `0% <0%> (-96.81%)` | :arrow_down: |
| [scrapy/linkextractors/regex.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmtleHRyYWN0b3JzL3JlZ2V4LnB5) | `0% <0%> (-95.66%)` | :arrow_down: |
| [scrapy/linkextractors/htmlparser.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmtleHRyYWN0b3JzL2h0bWxwYXJzZXIucHk=) | `0% <0%> (-92.07%)` | :arrow_down: |
| [scrapy/core/downloader/handlers/s3.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci9oYW5kbGVycy9zMy5weQ==) | `62.9% <0%> (-32.26%)` | :arrow_down: |
| [scrapy/extensions/statsmailer.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvc3RhdHNtYWlsZXIucHk=) | `0% <0%> (-30.44%)` | :arrow_down: |
| [scrapy/utils/boto.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL2JvdG8ucHk=) | `46.66% <0%> (-26.67%)` | :arrow_down: |
| [scrapy/\_monkeypatches.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L19tb25rZXlwYXRjaGVzLnB5) | `42.85% <0%> (-14.29%)` | :arrow_down: |
| [scrapy/link.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmsucHk=) | `86.36% <0%> (-13.64%)` | :arrow_down: |
| [scrapy/core/downloader/tls.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci90bHMucHk=) | `75.92% <0%> (-12.97%)` | :arrow_down: |
| ... and [20 more](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree-more) | |

codecov[bot]: # [Codecov](https://codecov.io/gh/scrapy/scrapy/pull/3899?src=pr&el=h1) Report
> Merging [#3899](https://codecov.io/gh/scrapy/scrapy/pull/3899?src=pr&el=desc) into [master](https://codecov.io/gh/scrapy/scrapy/commit/9c514b976ffdf069b81c4b7728a7e8e531710680?src=pr&el=desc) will **decrease** coverage by `3.04%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master    #3899      +/-   ##
==========================================
- Coverage   85.47%   82.42%   -3.05%     
==========================================
  Files         165      165              
  Lines        9624     9629       +5     
  Branches     1446     1447       +1     
==========================================
- Hits         8226     7937     -289     
- Misses       1145     1423     +278     
- Partials      253      269      +16
```

| [Impacted Files](https://codecov.io/gh/scrapy/scrapy/pull/3899?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [scrapy/loader/\_\_init\_\_.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xvYWRlci9fX2luaXRfXy5weQ==) | `95.51% <100%> (+0.14%)` | :arrow_up: |
| [scrapy/linkextractors/sgml.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmtleHRyYWN0b3JzL3NnbWwucHk=) | `0% <0%> (-96.81%)` | :arrow_down: |
| [scrapy/linkextractors/regex.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmtleHRyYWN0b3JzL3JlZ2V4LnB5) | `0% <0%> (-95.66%)` | :arrow_down: |
| [scrapy/linkextractors/htmlparser.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmtleHRyYWN0b3JzL2h0bWxwYXJzZXIucHk=) | `0% <0%> (-92.07%)` | :arrow_down: |
| [scrapy/core/downloader/handlers/s3.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci9oYW5kbGVycy9zMy5weQ==) | `62.9% <0%> (-32.26%)` | :arrow_down: |
| [scrapy/extensions/statsmailer.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2V4dGVuc2lvbnMvc3RhdHNtYWlsZXIucHk=) | `0% <0%> (-30.44%)` | :arrow_down: |
| [scrapy/utils/boto.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L3V0aWxzL2JvdG8ucHk=) | `46.66% <0%> (-26.67%)` | :arrow_down: |
| [scrapy/\_monkeypatches.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L19tb25rZXlwYXRjaGVzLnB5) | `42.85% <0%> (-14.29%)` | :arrow_down: |
| [scrapy/link.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2xpbmsucHk=) | `86.36% <0%> (-13.64%)` | :arrow_down: |
| [scrapy/core/downloader/tls.py](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree#diff-c2NyYXB5L2NvcmUvZG93bmxvYWRlci90bHMucHk=) | `75.92% <0%> (-12.97%)` | :arrow_down: |
| ... and [20 more](https://codecov.io/gh/scrapy/scrapy/pull/3899/diff?src=pr&el=tree-more) | |


Patch:
diff --git a/docs/topics/loaders.rst b/docs/topics/loaders.rst
index 12a5e5c60..81c8dab03 100644
--- a/docs/topics/loaders.rst
+++ b/docs/topics/loaders.rst
@@ -142,20 +142,6 @@ accept one (and only one) positional argument, which will be an iterable.
    containing the collected values (for that field). The result of the output
    processors is the value that will be finally assigned to the item.
 
-If you want to use a plain function as a processor, make sure it receives
-``self`` as the first argument::
-
-    def lowercase_processor(self, values):
-        for v in values:
-            yield v.lower()
-
-    class MyItemLoader(ItemLoader):
-        name_in = lowercase_processor
-
-This is because whenever a function is assigned as a class variable, it becomes
-a method and would be passed the instance as the the first argument when being
-called. See `this answer on stackoverflow`_ for more details.
-
 The other thing you need to keep in mind is that the values returned by input
 processors are collected internally (in lists) and then passed to output
 processors to populate the fields.
@@ -163,7 +149,7 @@ processors to populate the fields.
 Last, but not least, Scrapy comes with some :ref:`commonly used processors
 <topics-loaders-available-processors>` built-in for convenience.
 
-.. _this answer on stackoverflow: https://stackoverflow.com/a/35322635
+
 
 Declaring Item Loaders
 ======================
diff --git a/scrapy/loader/__init__.py b/scrapy/loader/__init__.py
index 60fd6d222..7cf67e29e 100644
--- a/scrapy/loader/__init__.py
+++ b/scrapy/loader/__init__.py
@@ -4,8 +4,7 @@ Item Loader
 See documentation in docs/topics/loaders.rst
 """
 from collections import defaultdict
-
-import six
+from contextlib import suppress
 
 from scrapy.item import Item
 from scrapy.loader.common import wrap_loader_context
@@ -15,6 +14,17 @@ from scrapy.utils.misc import arg_to_iter, extract_regex
 from scrapy.utils.python import flatten
 
 
+def unbound_method(method):
+    """
+    Allow to use single-argument functions as input or output processors
+    (no need to define an unused first 'self' argument)
+    """
+    with suppress(AttributeError):
+        if '.' not in method.__qualname__:
+            return method.__func__
+    return method
+
+
 class ItemLoader(object):
 
     default_item_class = Item
@@ -72,7 +82,7 @@ class ItemLoader(object):
         if value is None:
             return
         if not field_name:
-            for k, v in six.iteritems(value):
+            for k, v in value.items():
                 self._add_value(k, v)
         else:
             self._add_value(field_name, value)
@@ -82,7 +92,7 @@ class ItemLoader(object):
         if value is None:
             return
         if not field_name:
-            for k, v in six.iteritems(value):
+            for k, v in value.items():
                 self._replace_value(k, v)
         else:
             self._replace_value(field_name, value)
@@ -142,14 +152,14 @@ class ItemLoader(object):
         if not proc:
             proc = self._get_item_field_attr(field_name, 'input_processor',
                                              self.default_input_processor)
-        return proc
+        return unbound_method(proc)
 
     def get_output_processor(self, field_name):
         proc = getattr(self, '%s_out' % field_name, None)
         if not proc:
             proc = self._get_item_field_attr(field_name, 'output_processor',
                                              self.default_output_processor)
-        return proc
+        return unbound_method(proc)
 
     def _process_input_value(self, field_name, value):
         proc = self.get_input_processor(field_name)


Test patch:
diff --git a/tests/test_loader.py b/tests/test_loader.py
index b87602809..6bfc31dbf 100644
--- a/tests/test_loader.py
+++ b/tests/test_loader.py
@@ -994,5 +994,53 @@ class SelectJmesTestCase(unittest.TestCase):
             )
 
 
+# Functions as processors
+
+def function_processor_strip(iterable):
+    return [x.strip() for x in iterable]
+
+
+def function_processor_upper(iterable):
+    return [x.upper() for x in iterable]
+
+
+class FunctionProcessorItem(Item):
+    foo = Field(
+        input_processor=function_processor_strip,
+        output_processor=function_processor_upper,
+    )
+
+
+class FunctionProcessorItemLoader(ItemLoader):
+    default_item_class = FunctionProcessorItem
+
+
+class FunctionProcessorDictLoader(ItemLoader):
+    default_item_class = dict
+    foo_in = function_processor_strip
+    foo_out = function_processor_upper
+
+
+class FunctionProcessorTestCase(unittest.TestCase):
+
+    def test_processor_defined_in_item(self):
+        lo = FunctionProcessorItemLoader()
+        lo.add_value('foo', '  bar  ')
+        lo.add_value('foo', ['  asdf  ', '  qwerty  '])
+        self.assertEqual(
+            dict(lo.load_item()),
+            {'foo': ['BAR', 'ASDF', 'QWERTY']}
+        )
+
+    def test_processor_defined_in_item_loader(self):
+        lo = FunctionProcessorDictLoader()
+        lo.add_value('foo', '  bar  ')
+        lo.add_value('foo', ['  asdf  ', '  qwerty  '])
+        self.assertEqual(
+            dict(lo.load_item()),
+            {'foo': ['BAR', 'ASDF', 'QWERTY']}
+        )
+
+
 if __name__ == "__main__":
     unittest.main()