Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve typing for Spider.parse(). #6274

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

wRAR
Copy link
Member

@wRAR wRAR commented Mar 6, 2024

Based on https://stackoverflow.com/questions/77160087/correct-typing-of-a-method-without-a-fixed-signature

Should remove most of the mypy errors for parse() in user spiders (if they use older mypy Spider.parse() will appear untyped so no override error either). Doesn't help with scrapy-poet's callbacks that narrow down Response to DummyResponse: scrapinghub/scrapy-poet#179

This should probably have typing tests, I'll do that later.

@wRAR wRAR added the typing label Mar 6, 2024
@wRAR wRAR added this to the Scrapy 2.12 milestone Mar 6, 2024
@wRAR wRAR marked this pull request as draft March 6, 2024 19:01
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Merging #6274 (84056f4) into master (861646f) will decrease coverage by 0.03%.
The diff coverage is 37.50%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6274      +/-   ##
==========================================
- Coverage   88.82%   88.79%   -0.03%     
==========================================
  Files         161      161              
  Lines       11863    11867       +4     
  Branches     1918     1919       +1     
==========================================
  Hits        10537    10537              
- Misses        979      982       +3     
- Partials      347      348       +1     
Files Coverage Δ
scrapy/commands/bench.py 100.00% <100.00%> (ø)
scrapy/spiders/__init__.py 88.23% <28.57%> (-5.52%) ⬇️


from scrapy.crawler import Crawler
from scrapy.settings import BaseSettings

CallbackT = Callable[Concatenate[Response, ...], Any]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be useful in other places as it's a general type for a spider callback. I didn't think about that further though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant