search

scrapinghub / scrapy-poet

Page Object pattern for Scrapy
BSD 3-Clause "New" or "Revised" License
119 stars 28 forks source link

Some callback annotations violate LSP #179

Open wRAR opened 10 months ago

wRAR commented 10 months ago

scrapy.Spider.parse has a Response type hint for the response argument, and custom callbacks may also have hints for response and for some kwargs, and then in subclasses the callback with the same name can have different hints for the same arguments. In the simplest case it's just response: DummyResponse.

All of these violate the Liskov substitution principle and mypy will produce errors for the overridden callbacks. This should already happen when the base class is user-defined and will also happen for any parse() with e.g. response: DummyResponse after Scrapy with py.typed is released.

I don't currently have any suggestions for resolving this.

kmike commented 10 months ago

I wonder if we can register DummyResponse as a Response subclass. Or maybe we should have a sepatate type for annotation in Scrapy itself, to communicate what is the interface of the Response which the callbacks can expect, if it's not the Response.

wRAR commented 10 months ago

DummyResponse is a Response subclass but you cannot narrow argument types in subclasses.

Gallaecio commented 8 months ago

Annotated[Response, "dummy"]? :slightly_smiling_face: