scrapy / itemadapter

Common interface for data container classes
BSD 3-Clause "New" or "Revised" License
60 stars 10 forks source link

ADAPTER_CLASSES may not supported appendleft #82

Open VMRuiz opened 2 months ago

VMRuiz commented 2 months ago

ADAPTER_CLASSES is defined as Iterable

https://github.com/scrapy/itemadapter/blob/98be1281ec11664da93f836861f81734382ce4ea/itemadapter/adapter.py#L271

However, in the docs it's suggested to use appendleft method which is not part of the Iterable API. This causes errors with tools like mypy:

error: "Iterable[Type[AdapterInterface]]" has no attribute "appendleft" [attr-defined]

Either the class type should be changed to List or other classes that may support appendleft or a different way to extend ADAPTER_CLASSES should be given.

elacuesta commented 2 months ago

ADAPTER_CLASSES was originally typed as a Deque, this was most recently changed in https://github.com/scrapy/itemadapter/issues/71 (https://github.com/scrapy/itemadapter/pull/74). The appendleft example works in the context of the default implementation, this is mentioned in the docs:

The default implementation uses a collections.deque to support efficient addition/deletion of adapters classes to both ends, but if you are deriving a subclass (see the section on extending itemadapter for additional information), any other iterable (e.g. list, tuple) will work.

I can think of the following, though there might be a more correct and compact solution:

ADAPTER_CLASSES: Union[Iterable[Type[AdapterInterface]], Deque[Type[AdapterInterface]]] = deque( 
Gallaecio commented 2 months ago

I was thinking we need to keep the typing as it is, and change the documentation example to something like:

ItemAdapter.ADAPTER_CLASSES = (CustomClass, *ItemAdapter.ADAPTER_CLASSES)