zytedata / zyte-common-items

Contains the common item definitions used in Zyte.
BSD 3-Clause "New" or "Revised" License
9 stars 6 forks source link

Processor changes #99

Closed Nykakin closed 1 month ago

Nykakin commented 2 months ago

We'd like to remove processors from our utils and replace them with zyte-common-items counterparts. There are some functionality missing, however.

Summary of changes:

kmike commented 2 months ago

Overall, 👍, much needed changes. Having e.g. an images processor is a must.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 0% with 45 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (cce3b94) to head (30baf8a). Report is 6 commits behind head on main.

Files Patch % Lines
zyte_common_items/processors.py 0.00% 43 Missing :warning:
zyte_common_items/pages/product.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #99 +/- ## ===================================== Coverage 0.00% 0.00% ===================================== Files 54 55 +1 Lines 2107 2160 +53 ===================================== - Misses 2107 2160 +53 ``` | [Files](https://app.codecov.io/gh/zytedata/zyte-common-items/pull/99?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zytedata) | Coverage Δ | | |---|---|---| | [zyte\_common\_items/pages/product.py](https://app.codecov.io/gh/zytedata/zyte-common-items/pull/99?src=pr&el=tree&filepath=zyte_common_items%2Fpages%2Fproduct.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zytedata#diff-enl0ZV9jb21tb25faXRlbXMvcGFnZXMvcHJvZHVjdC5weQ==) | `0.00% <0.00%> (ø)` | | | [zyte\_common\_items/processors.py](https://app.codecov.io/gh/zytedata/zyte-common-items/pull/99?src=pr&el=tree&filepath=zyte_common_items%2Fprocessors.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=zytedata#diff-enl0ZV9jb21tb25faXRlbXMvcHJvY2Vzc29ycy5weQ==) | `0.00% <0.00%> (ø)` | |
Nykakin commented 2 months ago

In response to https://github.com/zytedata/zyte-common-items/pull/99#discussion_r1725091519 and after some discussion in slack I've decided to just return strings without any change in price_processor instead of parsing it, thus retaining original philosophy of transparent processors.

The discussion about this approach can continue but I do not want to block image processors and brand processor because of it.

kmike commented 1 month ago

Thanks @Nykakin!