Merge pull request #4534 from elacuesta/deprecate-baseitem

Deprecate scrapy.item.BaseItem

docs/faq.rst

@@ -342,14 +342,14 @@ method for this purpose. For example::
 
     from copy import deepcopy
 
-    from scrapy.item import BaseItem
+    from scrapy.item import Item
 
 
     class MultiplyItemsMiddleware:
 
         def process_spider_output(self, response, result, spider):
             for item in result:
-                if isinstance(item, (BaseItem, dict)):
+                if isinstance(item, (Item, dict)):
                     for _ in range(item['multiply_by']):
                         yield deepcopy(item)
 

docs/topics/items.rst

@@ -257,6 +257,4 @@ Field objects
 Other classes related to Item
 =============================
 
-.. autoclass:: BaseItem
-
 .. autoclass:: ItemMeta

scrapy/commands/parse.py

@@ -5,7 +5,7 @@ from w3lib.url import is_url
 
 from scrapy.commands import ScrapyCommand
 from scrapy.http import Request
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.utils import display
 from scrapy.utils.conf import arglist_to_dict
 from scrapy.utils.spider import iterate_spider_output, spidercls_for_request
@@ -117,7 +117,7 @@ class Command(ScrapyCommand):
         items, requests = [], []
 
         for x in iterate_spider_output(callback(response, **cb_kwargs)):
-            if isinstance(x, (BaseItem, dict)):
+            if isinstance(x, (_BaseItem, dict)):
                 items.append(x)
             elif isinstance(x, Request):
                 requests.append(x)

scrapy/contracts/default.py

@@ -1,6 +1,6 @@
 import json
 
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.http import Request
 from scrapy.exceptions import ContractFail
 
@@ -51,8 +51,8 @@ class ReturnsContract(Contract):
     objects = {
         'request': Request,
         'requests': Request,
-        'item': (BaseItem, dict),
-        'items': (BaseItem, dict),
+        'item': (_BaseItem, dict),
+        'items': (_BaseItem, dict),
     }
 
     def __init__(self, *args, **kwargs):
@@ -103,7 +103,7 @@ class ScrapesContract(Contract):
 
     def post_process(self, output):
         for x in output:
-            if isinstance(x, (BaseItem, dict)):
+            if isinstance(x, (_BaseItem, dict)):
                 missing = [arg for arg in self.args if arg not in x]
                 if missing:
                     raise ContractFail(

scrapy/core/scraper.py

@@ -14,7 +14,7 @@ from scrapy.utils.log import logformatter_adapter, failure_to_exc_info
 from scrapy.exceptions import CloseSpider, DropItem, IgnoreRequest
 from scrapy import signals
 from scrapy.http import Request, Response
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.core.spidermw import SpiderMiddlewareManager
 
 
@@ -191,7 +191,7 @@ class Scraper:
         """
         if isinstance(output, Request):
             self.crawler.engine.crawl(request=output, spider=spider)
-        elif isinstance(output, (BaseItem, dict)):
+        elif isinstance(output, (_BaseItem, dict)):
             self.slot.itemproc_size += 1
             dfd = self.itemproc.process_item(output, spider)
             dfd.addBoth(self._itemproc_finished, output, response, spider)

scrapy/exporters.py

@@ -12,7 +12,7 @@ from xml.sax.saxutils import XMLGenerator
 
 from scrapy.utils.serialize import ScrapyJSONEncoder
 from scrapy.utils.python import to_bytes, to_unicode, is_listlike
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.exceptions import ScrapyDeprecationWarning
 
 
@@ -312,7 +312,7 @@ class PythonItemExporter(BaseItemExporter):
         return serializer(value)
 
     def _serialize_value(self, value):
-        if isinstance(value, BaseItem):
+        if isinstance(value, _BaseItem):
             return self.export_item(value)
         if isinstance(value, dict):
             return dict(self._serialize_dict(value))

scrapy/item.py

@@ -14,28 +14,39 @@ from scrapy.utils.deprecate import ScrapyDeprecationWarning
 from scrapy.utils.trackref import object_ref
 
 
-class BaseItem(object_ref):
-    """Base class for all scraped items.
-
-    In Scrapy, an object is considered an *item* if it is an instance of either
-    :class:`BaseItem` or :class:`dict`. For example, when the output of a
-    spider callback is evaluated, only instances of :class:`BaseItem` or
-    :class:`dict` are passed to :ref:`item pipelines <topics-item-pipeline>`.
-
-    If you need instances of a custom class to be considered items by Scrapy,
-    you must inherit from either :class:`BaseItem` or :class:`dict`.
-
-    Unlike instances of :class:`dict`, instances of :class:`BaseItem` may be
-    :ref:`tracked <topics-leaks-trackrefs>` to debug memory leaks.
+class _BaseItem(object_ref):
+    """
+    Temporary class used internally to avoid the deprecation
+    warning raised by isinstance checks using BaseItem.
     """
     pass
 
 
+class _BaseItemMeta(ABCMeta):
+    def __instancecheck__(cls, instance):
+        if cls is BaseItem:
+            warn('scrapy.item.BaseItem is deprecated, please use scrapy.item.Item instead',
+                 ScrapyDeprecationWarning, stacklevel=2)
+        return super().__instancecheck__(instance)
+
+
+class BaseItem(_BaseItem, metaclass=_BaseItemMeta):
+    """
+    Deprecated, please use :class:`scrapy.item.Item` instead
+    """
+
+    def __new__(cls, *args, **kwargs):
+        if issubclass(cls, BaseItem) and not (issubclass(cls, Item) or issubclass(cls, DictItem)):
+            warn('scrapy.item.BaseItem is deprecated, please use scrapy.item.Item instead',
+                 ScrapyDeprecationWarning, stacklevel=2)
+        return super(BaseItem, cls).__new__(cls, *args, **kwargs)
+
+
 class Field(dict):
     """Container of field metadata"""
 
 
-class ItemMeta(ABCMeta):
+class ItemMeta(_BaseItemMeta):
     """Metaclass_ of :class:`Item` that handles field definitions.
 
     .. _metaclass: https://realpython.com/python-metaclasses
@@ -68,8 +79,7 @@ class DictItem(MutableMapping, BaseItem):
 
     def __new__(cls, *args, **kwargs):
         if issubclass(cls, DictItem) and not issubclass(cls, Item):
-            warn('scrapy.item.DictItem is deprecated, please use '
-                 'scrapy.item.Item instead',
+            warn('scrapy.item.DictItem is deprecated, please use scrapy.item.Item instead',
                  ScrapyDeprecationWarning, stacklevel=2)
         return super(DictItem, cls).__new__(cls, *args, **kwargs)
 
@@ -125,4 +135,24 @@ class DictItem(MutableMapping, BaseItem):
 
 
 class Item(DictItem, metaclass=ItemMeta):
-    pass
+    """
+    Base class for scraped items.
+
+    In Scrapy, an object is considered an ``item`` if it is an instance of either
+    :class:`Item` or :class:`dict`, or any subclass. For example, when the output of a
+    spider callback is evaluated, only instances of :class:`Item` or
+    :class:`dict` are passed to :ref:`item pipelines <topics-item-pipeline>`.
+
+    If you need instances of a custom class to be considered items by Scrapy,
+    you must inherit from either :class:`Item` or :class:`dict`.
+
+    Items must declare :class:`Field` attributes, which are processed and stored
+    in the ``fields`` attribute. This restricts the set of allowed field names
+    and prevents typos, raising ``KeyError`` when referring to undefined fields.
+    Additionally, fields can be used to define metadata and control the way
+    data is processed internally. Please refer to the :ref:`documentation
+    about fields <topics-items-fields>` for additional information.
+
+    Unlike instances of :class:`dict`, instances of :class:`Item` may be
+    :ref:`tracked <topics-leaks-trackrefs>` to debug memory leaks.
+    """

scrapy/shell.py

@@ -13,7 +13,7 @@ from w3lib.url import any_to_uri
 from scrapy.crawler import Crawler
 from scrapy.exceptions import IgnoreRequest
 from scrapy.http import Request, Response
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 from scrapy.settings import Settings
 from scrapy.spiders import Spider
 from scrapy.utils.console import start_python_console
@@ -26,8 +26,7 @@ from scrapy.utils.console import DEFAULT_PYTHON_SHELLS
 
 class Shell:
 
-    relevant_classes = (Crawler, Spider, Request, Response, BaseItem,
-                        Settings)
+    relevant_classes = (Crawler, Spider, Request, Response, _BaseItem, Settings)
 
     def __init__(self, crawler, update_vars=None, code=None):
         self.crawler = crawler

scrapy/spiders/feed.py

@@ -52,7 +52,7 @@ class XMLFeedSpider(Spider):
         """This method is called for the nodes matching the provided tag name
         (itertag). Receives the response and an Selector for each node.
         Overriding this method is mandatory. Otherwise, you spider won't work.
-        This method must return either a BaseItem, a Request, or a list
+        This method must return either an item, a request, or a list
         containing any of them.
         """
 

scrapy/utils/misc.py

@@ -14,10 +14,10 @@ from w3lib.html import replace_entities
 
 from scrapy.utils.datatypes import LocalWeakReferencedCache
 from scrapy.utils.python import flatten, to_unicode
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 
 
-_ITERABLE_SINGLE_VALUES = dict, BaseItem, str, bytes
+_ITERABLE_SINGLE_VALUES = dict, _BaseItem, str, bytes
 
 
 def arg_to_iter(arg):

scrapy/utils/serialize.py

@@ -5,7 +5,7 @@ import decimal
 from twisted.internet import defer
 
 from scrapy.http import Request, Response
-from scrapy.item import BaseItem
+from scrapy.item import _BaseItem
 
 
 class ScrapyJSONEncoder(json.JSONEncoder):
@@ -26,7 +26,7 @@ class ScrapyJSONEncoder(json.JSONEncoder):
             return str(o)
         elif isinstance(o, defer.Deferred):
             return str(o)
-        elif isinstance(o, BaseItem):
+        elif isinstance(o, _BaseItem):
             return dict(o)
         elif isinstance(o, Request):
             return "<%s %s %s>" % (type(o).__name__, o.method, o.url)

tests/test_item.py

@@ -4,7 +4,7 @@ from unittest import mock
 from warnings import catch_warnings
 
 from scrapy.exceptions import ScrapyDeprecationWarning
-from scrapy.item import ABCMeta, DictItem, Field, Item, ItemMeta
+from scrapy.item import ABCMeta, _BaseItem, BaseItem, DictItem, Field, Item, ItemMeta
 
 
 PY36_PLUS = (sys.version_info.major >= 3) and (sys.version_info.minor >= 6)
@@ -131,12 +131,12 @@ class ItemTest(unittest.TestCase):
         self.assertSortedEqual(list(item.values()), [u'New'])
 
     def test_metaclass_inheritance(self):
-        class BaseItem(Item):
+        class ParentItem(Item):
             name = Field()
             keys = Field()
             values = Field()
 
-        class TestItem(BaseItem):
+        class TestItem(ParentItem):
             keys = Field()
 
         i = TestItem()
@@ -330,5 +330,77 @@ class DictItemTest(unittest.TestCase):
             self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
 
 
+class BaseItemTest(unittest.TestCase):
+
+    def test_isinstance_check(self):
+
+        class SubclassedBaseItem(BaseItem):
+            pass
+
+        class SubclassedItem(Item):
+            pass
+
+        self.assertTrue(isinstance(BaseItem(), BaseItem))
+        self.assertTrue(isinstance(SubclassedBaseItem(), BaseItem))
+        self.assertTrue(isinstance(Item(), BaseItem))
+        self.assertTrue(isinstance(SubclassedItem(), BaseItem))
+
+        # make sure internal checks using private _BaseItem class succeed
+        self.assertTrue(isinstance(BaseItem(), _BaseItem))
+        self.assertTrue(isinstance(SubclassedBaseItem(), _BaseItem))
+        self.assertTrue(isinstance(Item(), _BaseItem))
+        self.assertTrue(isinstance(SubclassedItem(), _BaseItem))
+
+    def test_deprecation_warning(self):
+        """
+        Make sure deprecation warnings are logged whenever BaseItem is used,
+        either instantiated or in an isinstance check
+        """
+        with catch_warnings(record=True) as warnings:
+            BaseItem()
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
+        with catch_warnings(record=True) as warnings:
+
+            class SubclassedBaseItem(BaseItem):
+                pass
+
+            SubclassedBaseItem()
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
+        with catch_warnings(record=True) as warnings:
+            self.assertFalse(isinstance("foo", BaseItem))
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
+        with catch_warnings(record=True) as warnings:
+            self.assertTrue(isinstance(BaseItem(), BaseItem))
+            self.assertEqual(len(warnings), 1)
+            self.assertEqual(warnings[0].category, ScrapyDeprecationWarning)
+
+
+class ItemNoDeprecationWarningTest(unittest.TestCase):
+    def test_no_deprecation_warning(self):
+        """
+        Make sure deprecation warnings are NOT logged whenever BaseItem subclasses are used.
+        """
+        class SubclassedItem(Item):
+            pass
+
+        with catch_warnings(record=True) as warnings:
+            Item()
+            SubclassedItem()
+            _BaseItem()
+            self.assertFalse(isinstance("foo", _BaseItem))
+            self.assertFalse(isinstance("foo", Item))
+            self.assertFalse(isinstance("foo", SubclassedItem))
+            self.assertTrue(isinstance(_BaseItem(), _BaseItem))
+            self.assertTrue(isinstance(Item(), Item))
+            self.assertTrue(isinstance(SubclassedItem(), SubclassedItem))
+            self.assertEqual(len(warnings), 0)
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_loader.py

@@ -601,7 +601,7 @@ class NoInputReprocessingItemLoader(BaseNoInputReprocessingLoader):
 
 class NoInputReprocessingFromItemTest(unittest.TestCase):
     """
-    Loaders initialized from loaded items must not reprocess fields (BaseItem instances)
+    Loaders initialized from loaded items must not reprocess fields (Item instances)
     """
     def test_avoid_reprocessing_with_initial_values_single(self):
         il = NoInputReprocessingItemLoader(item=NoInputReprocessingItem(title='foo'))

tests/test_utils_spider.py

@@ -2,7 +2,7 @@ import unittest
 
 from scrapy import Spider
 from scrapy.http import Request
-from scrapy.item import BaseItem
+from scrapy.item import Item
 from scrapy.utils.spider import iterate_spider_output, iter_spider_classes
 
 
@@ -17,7 +17,7 @@ class MySpider2(Spider):
 class UtilsSpidersTestCase(unittest.TestCase):
 
     def test_iterate_spider_output(self):
-        i = BaseItem()
+        i = Item()
         r = Request('http://scrapytest.org')
         o = object()