Repository: BurnzZ/scrapy-loader-upkeep

Issue:
Allow paths to be named
When having several amounts of xpaths for each field, is really difficult to determine which one was added firsts, specially when there are different workflows on the code.

To make easier the task to identify the paths on the stats, it would be great if we could name them. For example:

```python
loader.add_css('NAME', 'h1::text', name='Name from h1')
loader.add_css('NAME', 'meta[value="title"]::attr(content)', name="Name from meta tag")
```



Discussion:

Patch:
diff --git a/README.rst b/README.rst
index 6fb0e87..3112d48 100644
--- a/README.rst
+++ b/README.rst
@@ -58,8 +58,8 @@ This only works for the following ``ItemLoader`` methods:
    - ``add_xpath()``
    - ``replace_xpath()``
 
-Spider Example
-~~~~~~~~~~~~~~
+Basic Spider Example
+~~~~~~~~~~~~~~~~~~~~
 This is taken from the `examples/ 
 <https://github.com/BurnzZ/scrapy-loader-upkeep/tree/master/examples>`_
 directory.
@@ -83,6 +83,77 @@ This should output in the stats:
 In this example, we could see that the **1st css** rule for the ``quote`` field
 has had instances of not being matched at all during the scrape.
 
+New Feature
+~~~~~~~~~~~
+
+As with the example above, we're limited only to the positional context of when
+the ``add_css()``, ``add_xpath()``, etc were called during the execution.
+
+There will be cases where developers will be maintaining a large spider with a
+lot of different parsers to handle varying layouts in the site. It would make
+sense to have a better context to what a parser does or is for.
+
+A new optional ``name`` parameter is supported to provide more context around a
+given parser. This supports the two (2) main types of creating fallback parsers:
+
+1. **multiple calls**
+
+.. code-block:: python
+
+   loader.add_css('NAME', 'h1::text', name='Name from h1')
+   loader.add_css('NAME', 'meta[value="title"]::attr(content)', name="Name from meta tag")
+
+would result in something like:
+
+.. code-block:: python
+
+   { ...
+     'parser/QuotesItemLoader/NAME/css/1/Name from h1': 8,
+     'parser/QuotesItemLoader/NAME/css/1/Name from h1/missing': 2,
+     'parser/QuotesItemLoader/NAME/css/2/Name from meta tag': 7,
+     'parser/QuotesItemLoader/NAME/css/2/Name from meta tag/missing': 3,
+     ...
+   }
+
+2. **grouped parsers in a single call**
+
+.. code-block:: python
+
+   loader.add_css(
+       'NAME',
+       [
+           'h1::text',
+           'meta[value="title"]::attr(content)',
+       ],
+       name='NAMEs at the main content')
+   loader.add_css(
+       'NAME',
+       [
+           'footer .name::text',
+           'div.page-end span.name::text',
+       ],
+       name='NAMEs at the bottom of the page')
+
+would result in something like:
+
+.. code-block:: python
+
+   { ...
+     'parser/QuotesItemLoader/NAME/css/1/NAMEs at the main content': 8,
+     'parser/QuotesItemLoader/NAME/css/1/NAMEs at the main content/missing': 2,
+     'parser/QuotesItemLoader/NAME/css/2/NAMEs at the main content': 7,
+     'parser/QuotesItemLoader/NAME/css/2/NAMEs at the main content/missing': 3,
+     'parser/QuotesItemLoader/NAME/css/3/NAMEs at the bottom of the page': 8,
+     'parser/QuotesItemLoader/NAME/css/3/NAMEs at the bottom of the page/missing': 2,
+     'parser/QuotesItemLoader/NAME/css/4/NAMEs at the bottom of the page': 7,
+     'parser/QuotesItemLoader/NAME/css/4/NAMEs at the bottom of the page/missing': 3,
+     ...
+   }
+
+The latter is useful in grouping fallback parsers together if they are quite
+related in terms of layout/arrangement in the page.
+
+
 Requirements
 ~~~~~~~~~~~~
 Python 3.6+
diff --git a/examples/README.rst b/examples/README.rst
index fc57fdb..037f806 100644
--- a/examples/README.rst
+++ b/examples/README.rst
@@ -1,5 +1,12 @@
 Contains examples of using the ``scrapy-loader-upkeep`` package inside Scrapy.
 
+A symlink has been included in the example project dir in order to read the
+source code. Run the following in order to test these out:
+
+.. code-block:: bash
+
+   $ python3 -m scrapy crawl <spider_name>
+
 
 Simple Example
 ~~~~~~~~~~~~~~
@@ -41,3 +48,24 @@ This should output in the stats:
      'parser/QuotesItemLoader/quote/css/2': 10
      ...
    }
+
+
+Example using 'name' feature
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. code-block:: bash
+
+   $ scrapy crawl quotestoscrape_use_name
+
+This should output in the stats:
+
+.. code-block:: python
+
+   2019-06-16 14:32:32 [scrapy.statscollectors] INFO: Dumping Scrapy stats:
+   { ...
+     'parser/QuotesItemLoader/author/css/1/basic author class': 10,
+     'parser/QuotesItemLoader/quote/css/1/Quotes inside the box/missing': 10,
+     'parser/QuotesItemLoader/quote/css/2/Quotes inside the box': 10,
+     'parser/QuotesItemLoader/tags/css/1/underneath the author text': 10,
+     ...
+   }
diff --git a/scrapy_loader_upkeep/loader.py b/scrapy_loader_upkeep/loader.py
index cb47f96..805b3ce 100644
--- a/scrapy_loader_upkeep/loader.py
+++ b/scrapy_loader_upkeep/loader.py
@@ -79,6 +79,9 @@ class ItemLoader(ItemLoaderOG):
 
         selector_type = selector.__name__  # either 'css' or 'xpath'
 
+        # The optional arg in methods like `add_css()` for context in stats
+        name = kw.get('name')
+
         # For every call of `add_css()` and `add_xpath()` this is incremented.
         # We'll use it as the base index of the position of the logged stats.
         index = self.field_tracker[f'{field_name}_{selector_type}']
@@ -87,10 +90,12 @@ class ItemLoader(ItemLoaderOG):
         for position, rule in enumerate(arg_to_iter(selector_rules), index):
             parsed_data = selector(rule).getall()
             values.append(parsed_data)
-            self.write_to_stats(field_name, parsed_data, position, selector_type)
+            self.write_to_stats(field_name, parsed_data, position,
+                                selector_type, name=name)
         return flatten(values)
 
-    def write_to_stats(self, field_name, parsed_data, position, selector_type):
+    def write_to_stats(self, field_name, parsed_data, position, selector_type,
+                       name=None):
         """Responsible for logging the parser rules usage.
 
         NOTES: It's hard to easily denote which parser rule hasn't produced any
@@ -111,6 +116,9 @@ class ItemLoader(ItemLoaderOG):
             f"parser/{self.loader_name}/{field_name}/{selector_type}/{position}"
         )
 
+        if name:
+            parser_label += f'/{name}'
+
         if parsed_data in (None, []):
             parser_label += "/missing"
 


Test patch:
diff --git a/examples/quotestoscrape/items.py b/examples/quotestoscrape/items.py
index 360d6f0..2eac930 100644
--- a/examples/quotestoscrape/items.py
+++ b/examples/quotestoscrape/items.py
@@ -3,3 +3,4 @@ import scrapy
 class QuotesToScrapeItem(scrapy.Item):
     quote = scrapy.Field()
     author = scrapy.Field()
+    tags = scrapy.Field()
diff --git a/examples/quotestoscrape/spiders/quotestoscrape.py b/examples/quotestoscrape/spiders/quotestoscrape.py
index 04dd31e..da8ff0e 100644
--- a/examples/quotestoscrape/spiders/quotestoscrape.py
+++ b/examples/quotestoscrape/spiders/quotestoscrape.py
@@ -5,7 +5,7 @@ from ..loader import QuotesItemLoader
 
 class BaseExampleSpider(scrapy.Spider):
 
-    start_urls = ['http://quotes.toscrape.com/']
+    start_urls = ["http://quotes.toscrape.com/"]
 
 
 class QuotesToScrapeSimpleSpider(BaseExampleSpider):
@@ -22,18 +22,17 @@ class QuotesToScrapeSimpleSpider(BaseExampleSpider):
     been successfully matched.
     """
 
-    name = 'quotestoscrape_simple'
-
+    name = "quotestoscrape_simple"
 
     def parse(self, response):
         """Notice that in order for this enhanced ItemLoader package to work,
         we'll need to inject the stats API into it.
         """
 
-        for quote_div in response.css('div.quote'):
+        for quote_div in response.css("div.quote"):
             loader = QuotesItemLoader(selector=quote_div, stats=self.crawler.stats)
-            loader.add_css('quote', '.quote > span[itemprop="text"]::text')
-            loader.add_css('author', '.author::text')
+            loader.add_css("quote", '.quote > span[itemprop="text"]::text')
+            loader.add_css("author", ".author::text")
             yield loader.load_item()
 
 
@@ -50,20 +49,58 @@ class QuotesToScrapeHasMissingSpider(BaseExampleSpider):
         }
     """
 
-    name = 'quotestoscrape_has_missing'
+    name = "quotestoscrape_has_missing"
+
+    def parse(self, response):
+        """Notice that in order for this enhanced ItemLoader package to work,
+        we'll need to inject the stats API into it.
+        """
+
+        for quote_div in response.css("div.quote"):
+            loader = QuotesItemLoader(selector=quote_div, stats=self.crawler.stats)
+            loader.add_css(
+                "quote",
+                [
+                    # This first parser rule doesn't exist at all.
+                    ".this-quote-does-not-exist span::text",
+                    '.quote > span[itemprop="text"]::text',
+                ],
+            )
+            loader.add_css("author", ".author::text")
+            yield loader.load_item()
+
+
+class QuotesToScrapeUseName(BaseExampleSpider):
+    """Demonstrates the new feature of naming the parsers using the 'name' param.
+
+        2019-06-16 14:32:32 [scrapy.statscollectors] INFO: Dumping Scrapy stats:
+        { ...
+          'parser/QuotesItemLoader/author/css/1/basic author class': 10,
+          'parser/QuotesItemLoader/quote/css/1/Quotes inside the box/missing': 10,
+          'parser/QuotesItemLoader/quote/css/2/Quotes inside the box': 10,
+          'parser/QuotesItemLoader/tags/css/1/underneath the author text': 10,
+          ...
+        }
+    """
 
+    name = "quotestoscrape_use_name"
 
     def parse(self, response):
         """Notice that in order for this enhanced ItemLoader package to work,
         we'll need to inject the stats API into it.
         """
 
-        for quote_div in response.css('div.quote'):
+        for quote_div in response.css("div.quote"):
             loader = QuotesItemLoader(selector=quote_div, stats=self.crawler.stats)
-            loader.add_css('quote', [
-                # This first parser rule doesn't exist at all.
-                '.this-quote-does-not-exist span::text',
-                '.quote > span[itemprop="text"]::text'
-            ])
-            loader.add_css('author', '.author::text')
+            loader.add_css(
+                "quote",
+                [
+                    # This first parser rule doesn't exist at all.
+                    ".this-quote-does-not-exist span::text",
+                    '.quote > span[itemprop="text"]::text',
+                ],
+                name="Quotes inside the box",
+            )
+            loader.add_css("author", ".author::text", name="basic author class")
+            loader.add_css("tags", ".tag::text", name="underneath the author text")
             yield loader.load_item()
diff --git a/tests/test_loader.py b/tests/test_loader.py
index 2a3672b..92a7f58 100644
--- a/tests/test_loader.py
+++ b/tests/test_loader.py
@@ -50,9 +50,9 @@ def test_get_selector_values():
 
     loader.write_to_stats.assert_has_calls(
         [
-            mock.call(field_name, parsed_data, 1, "css"),
-            mock.call(field_name, parsed_data, 2, "css"),
-            mock.call(field_name, parsed_data, 3, "css"),
+            mock.call(field_name, parsed_data, 1, "css", name=None),
+            mock.call(field_name, parsed_data, 2, "css", name=None),
+            mock.call(field_name, parsed_data, 3, "css", name=None),
         ]
     )
 
@@ -205,16 +205,36 @@ def test_multiple_1(loader):
             mock.call("parser/TestItemLoader/title/css/3"),
         ]
     )
+    assert loader.stats.inc_value.call_count == 3
 
 
-def test_multiple_2(loader):
-    loader.add_css('title', 'h2::text')
-    loader.add_xpath('title', '//article/h2/text()')
+def test_multiple_1_with_name(loader):
+    loader.add_css('title', 'h2::text', name='title from h2')
+    loader.add_css('title', [
+        'article h2::text',
+        'article .product-title::text',
+    ], name='title from article')
+    loader.stats.inc_value.assert_has_calls(
+        [
+            mock.call("parser/TestItemLoader/title/css/1/title from h2"),
+            mock.call("parser/TestItemLoader/title/css/2/title from article"),
+            mock.call("parser/TestItemLoader/title/css/3/title from article"),
+        ]
+    )
+    assert loader.stats.inc_value.call_count == 3
+
+
+def test_multiple_2_with_name(loader):
+    loader.add_css('title', 'h2::text', name='title from h2')
+    loader.add_xpath('title', '//article/h2/text()', name='title from article')
     loader.add_css('title', 'article .product-title::text')
+    loader.add_xpath('title', '//aside/h1/text()', name='title from aside')
     loader.stats.inc_value.assert_has_calls(
         [
-            mock.call("parser/TestItemLoader/title/css/1"),
-            mock.call("parser/TestItemLoader/title/xpath/1"),
+            mock.call("parser/TestItemLoader/title/css/1/title from h2"),
+            mock.call("parser/TestItemLoader/title/xpath/1/title from article"),
             mock.call("parser/TestItemLoader/title/css/2"),
+            mock.call("parser/TestItemLoader/title/xpath/2/title from aside/missing"),
         ]
     )
+    assert loader.stats.inc_value.call_count == 4