zytedata / zyte-common-items Goto Github PK

There are some modules which imports attr instead of attrs.

Currently tests don't collect coverage data and print a warning about that. Running pytest directly works while running tox doesn't. I feel like it's because of the path problem currently hidden by PY_IGNORE_IMPORTMISMATCH=1 (as coverage collection should depend on file paths), but I don't know how to fix it.

Standard price processors at the from ProductPage don't process integer and float values.

description:
Supported inputs are Selector, SelectorList and HtmlElement. Other inputs are returned as is.

https://zyte-common-items.readthedocs.io/en/latest/reference/field-processors.html#zyte_common_items.processors.price_processor

nodes decorator:
https://github.com/zytedata/zyte-common-items/blob/main/zyte_common_items/processors.py#L118

mypy 1.5.0 has an additional warning:

zyte_common_items/base.py:53: error: Trying to assign name "_unknown_fields_dict" that is not in "__slots__" of type "zyte_common_items.base.Item"  [misc]
zyte_common_items/base.py:77: error: Trying to assign name "_unknown_fields_dict" that is not in "__slots__" of type "zyte_common_items.base.Item"  [misc]

Item doesn't define __slots__ but its base class _ItemBase does (and it includes _unknown_fields_dict). _ItemBase is a normal class, Item is decorated with @attrs.define. There are some entries about attr classes and __slots__ in the mypy 1.5.0 changelog, but I don't see if it's a false positive or a newly detected problem in our code. python/mypy#15639 is likely the change that causes this.

Also related:

https://mypy.readthedocs.io/en/stable/class_basics.html#slots

https://www.attrs.org/en/stable/examples.html#slots

https://www.attrs.org/en/stable/glossary.html#term-slotted-classes

Stemming off from the discussion in #15.

We need a library of functions that can be used to preprocess the data values placed into the item. Ideally, this should be used by https://github.com/scrapinghub/web-poet fields. For example:

from web_poet import ItemPage, field
from zyte_common_items import Product
from zyte_common_items.processors import clean_str


class ProductPage(ItemPage[Product]):
    @field
    @clean_str
    def name(self):
        return "  some value \n\n\t"

page = ProductPage()
assert page.name == "some value"
assert page.name == page.to_item().name

A guideline in web-poet should be created as well to properly document the processors found in zyte-common-items.

zyte_common_items.util.convert_to_class() expects an attrs-class and so returning a dict from metadata() is no longer supported.

Currently, items are declared like:

import attrs
from zyte_common_items.util import export

@export
@attrs.define(slots=True, kw_only=True)
class SomeItem:
    ...

It would be nice if we can combine the two decorators into a single one to have a more concise code.

It looks like some of the files grew too much, e.g. items.py, or even components.py. I think it makes sense to split them.

Currently we override line length to be 120. I'm not sure what's the reason for this, and propose to use black defaults.

This fails:

def test_metadata_with_returns():
    class MyProduct(Product):
        pass

    class CustomProductPage(ProductPage, Returns[MyProduct]):
        pass

    url = ResponseUrl("https://example.com")
    html = b""
    page = CustomProductPage(response=HttpResponse(url=url, body=html))
    metadata = page.metadata

    @field
    def metadata(self) -> MetadataT:
>       value = self.metadata_cls()
E       TypeError: 'NoneType' object is not callable

zyte_common_items/pages.py:67: TypeError

zyte_common_items.pages._get_metadata_class in the case of this CustomProductPage cannot find any HasMetadata base and returns None.