New ItemPage

[kmike]

This PR is built on top of #62 and #70, and should be only considered for merging after these 2 PRs. It could also make sense to review those 2 first.

The idea here is to extend ItemPage class, so that it provides a default to_item implementation, which uses web_poet fields if they're available.

By default, dicts are created, but ItemPage can be parametrized with the item type.

class ProductPage(ItemPage[Product]):
    pass

In the example above, await product_page.to_item() would output a Product item, not a dict. Mypy is inferring types correctly for this case, i.e. it knows that item would be Product.

It's possible to change the item type in a subclass, by inheriting from Returns[MyItem] (see the tutorial). This feature works from the output point of view, but I was unable to make typing work properly in this case (commented inline). It could be a mypy bug though, I'm not sure.

Use cases with extended items (adding new fields) and with custom items (removing / changing fields) are covered in tests. There is skip_nonitem_fields=True argument exposed for the latter use case. #63 is a related PR which covers a remaining case (extra fields in the base class), but I'm not sure if that's a good pattern or not; I think it's better to discuss extra fields in the base classes separately..

The change is almost backwards compatible. I don't expect it to break any code, but it might break some type validation using mypy. There are 2 slight incompatibilities:

1. abstractmethod validation for to_item method is removed, because there is a default implementation now. It shouldn't affect user code.
2. to_item is defined as async. Code-wise, it shouldn't break user code, because existing ItemPage subclasses must implement their own to_item methods, and I can't imagine why they might call super().to_item(). It might break typing though, because mypy may issue an error (or a warning, not sure), if a regular def to_item(self) is defined in ItemPage subclass.

TODO:

• decide if we should rename item_cls_fields parameter, and follow the decision
• API docs
• Make sure Returns is in API docs
• Rewrite fields tutorial
• WebPage should supports fields. Should we deprecate ItemWebPage?
• Check that other tutorials which mention ItemPage are valid

[codecov]

Codecov Report

Merging #74 (28f917a) into master (944dcd7) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #74      +/-   ##
==========================================
+ Coverage   99.80%   99.81%   +0.01%     
==========================================
  Files          17       18       +1     
  Lines         509      538      +29     
==========================================
+ Hits          508      537      +29     
  Misses          1        1              

Impacted Files	Coverage Δ
web_poet/overrides.py	100.00% <ø> (ø)
web_poet/__init__.py	100.00% <100.00%> (ø)
web_poet/_typing.py	100.00% <100.00%> (ø)
web_poet/fields.py	98.33% <100.00%> (+0.02%)	⬆️
web_poet/pages.py	100.00% <100.00%> (ø)

[kmike]

It seems this can be made a "class property", but it requires a bit more work

[kmike]

I'd propose to have it out of scope for the current PR>

[BurnzZ]

Suggested change

	Alternatively, you can use ``skip_nonitem_fields=True`` class argument - it tells
	Alternatively, you can use ``skip_nonitem_fields=True`` class argument (*default*: ``False``) - it tells

[kmike]

I'm on fence about this suggestion :) On one hand, it makes sense. On the other hand, class arguments are so rarely used, so that thinking about default values of class arguments could be too much. I think for most users it'd be just "copy-paste skip_nonitem_fields=True". No strong opinon on it though.

[BurnzZ]

🚀

[kmike]

Thanks for the review @Gallaecio @BurnzZ!