Repository: scrapinghub/web-poet

Issue:
`skip_nonitem_fields=True` doesn't work when the page object is an attrs class
Currently, this works fine:

```python
import attrs
from web_poet import HttpResponse, Returns, ItemPage, field

@attrs.define
class BigItem:
    x: int
    y: int

class BigPage(ItemPage[BigItem]):
    @field
    def x(self):
        return 1

    @field
    def y(self):
        return 2

@attrs.define
class SmallItem:
    x: int

class SmallXPage(BigPage, Returns[SmallItem], skip_nonitem_fields=True):
    pass

page = SmallXPage()
item = await page.to_item()

print(page._skip_nonitem_fields)  # True
print(item)  # SmallItem(x=1)
```

However, if we define an attrs class to have some page dependencies, it doesn't work:

```python
from web_poet import PageParams

@attrs.define
class SmallPage(BigPage, Returns[SmallItem], skip_nonitem_fields=True):
    params: PageParams

page = SmallPage(params=PageParams())
print(page._skip_nonitem_fields)  # False
item = await page.to_item()  # TypeError: __init__() got an unexpected keyword argument 'y'
```

From the examples above, this stems from `page._skip_nonitem_fields` being set to `False` when the page object is defined as an attrs class.

Discussion:

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index c88ec58..5ab56e4 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -2,6 +2,15 @@
 Changelog
 =========
 
+TBR
+---
+
+* Fix the error when calling :meth:`.to_item() <web_poet.pages.ItemPage.to_item>`,
+  :func:`item_from_fields_sync() <web_poet.fields.item_from_fields_sync>`, or
+  :func:`item_from_fields() <web_poet.fields.item_from_fields>` on page objects
+  defined as slotted attrs classes, while setting ``skip_nonitem_fields=True``.
+
+
 0.8.0 (2023-02-23)
 ------------------
 
diff --git a/web_poet/pages.py b/web_poet/pages.py
index 77b39af..fed2fae 100644
--- a/web_poet/pages.py
+++ b/web_poet/pages.py
@@ -50,21 +50,34 @@ class Returns(typing.Generic[ItemT]):
         return get_item_cls(self.__class__, default=dict)
 
 
+_NOT_SET = object()
+
+
 class ItemPage(Injectable, Returns[ItemT]):
     """Base Page Object, with a default :meth:`to_item` implementation
     which supports web-poet fields.
     """
 
-    _skip_nonitem_fields: bool
+    _skip_nonitem_fields = _NOT_SET
+
+    def _get_skip_nonitem_fields(self) -> bool:
+        value = self._skip_nonitem_fields
+        return False if value is _NOT_SET else bool(value)
 
-    def __init_subclass__(cls, skip_nonitem_fields: bool = False, **kwargs):
+    def __init_subclass__(cls, skip_nonitem_fields=_NOT_SET, **kwargs):
         super().__init_subclass__(**kwargs)
+        if skip_nonitem_fields is _NOT_SET:
+            # This is a workaround for attrs issue.
+            # See: https://github.com/scrapinghub/web-poet/issues/141
+            return
         cls._skip_nonitem_fields = skip_nonitem_fields
 
     async def to_item(self) -> ItemT:
         """Extract an item from a web page"""
         return await item_from_fields(
-            self, item_cls=self.item_cls, skip_nonitem_fields=self._skip_nonitem_fields
+            self,
+            item_cls=self.item_cls,
+            skip_nonitem_fields=self._get_skip_nonitem_fields(),
         )
 
 


Test patch:
diff --git a/tests/test_pages.py b/tests/test_pages.py
index fa3cf8d..d54f094 100644
--- a/tests/test_pages.py
+++ b/tests/test_pages.py
@@ -3,7 +3,7 @@ from typing import Optional
 import attrs
 import pytest
 
-from web_poet import HttpResponse, field
+from web_poet import HttpResponse, PageParams, field
 from web_poet.pages import (
     Injectable,
     ItemPage,
@@ -199,11 +199,27 @@ async def test_item_page_change_item_type_remove_fields() -> None:
     class Subclass(BasePage, Returns[Item], skip_nonitem_fields=True):
         pass
 
-    page = Subclass()
+    # Same as above but a slotted attrs class with dependency.
+    # See: https://github.com/scrapinghub/web-poet/issues/141
+    @attrs.define
+    class SubclassWithDep(BasePage, Returns[Item], skip_nonitem_fields=True):
+        params: PageParams
+
+    # Check if flicking skip_nonitem_fields to False in the subclass works
+    @attrs.define
+    class SubclassSkipFalse(SubclassWithDep, Returns[Item], skip_nonitem_fields=False):
+        pass
+
+    for page in [Subclass(), SubclassWithDep(params=PageParams())]:
+        assert page.item_cls is Item
+        item = await page.to_item()
+        assert isinstance(item, Item)
+        assert item == Item(name="hello")
+
+    page = SubclassSkipFalse(params=PageParams())
     assert page.item_cls is Item
-    item = await page.to_item()
-    assert isinstance(item, Item)
-    assert item == Item(name="hello")
+    with pytest.raises(TypeError, match="unexpected keyword argument 'price'"):
+        await page.to_item()
 
     # Item only contains "name", but not "price", but "price" should be passed
     class SubclassStrict(BasePage, Returns[Item]):