Skip to content

Typing for scrapy/utils, second pass #6003

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

Merged
merged 15 commits into from
Aug 9, 2023
Merged

Conversation

wRAR
Copy link
Member

@wRAR wRAR commented Aug 6, 2023

No description provided.

@codecov
Copy link

codecov bot commented Aug 6, 2023

Codecov Report

Merging #6003 (9cba193) into master (110d5ff) will decrease coverage by 0.16%.
The diff coverage is 86.19%.

❗ Current head 9cba193 differs from pull request most recent head 8050257. Consider uploading reports for the commit 8050257 to get more accurate results

@@            Coverage Diff             @@
##           master    #6003      +/-   ##
==========================================
- Coverage   89.24%   89.08%   -0.16%     
==========================================
  Files         162      162              
  Lines       11325    11425     +100     
  Branches     1835     1856      +21     
==========================================
+ Hits        10107    10178      +71     
- Misses        927      949      +22     
- Partials      291      298       +7     
Files Changed Coverage Δ
scrapy/contracts/__init__.py 82.22% <60.00%> (-1.38%) ⬇️
scrapy/utils/trackref.py 76.92% <66.66%> (-6.42%) ⬇️
scrapy/utils/spider.py 75.00% <71.42%> (-5.00%) ⬇️
scrapy/http/response/text.py 98.42% <77.77%> (-1.58%) ⬇️
scrapy/extensions/feedexport.py 95.05% <81.81%> (-0.56%) ⬇️
scrapy/utils/defer.py 96.20% <81.81%> (-1.12%) ⬇️
scrapy/utils/iterators.py 92.43% <84.78%> (-5.51%) ⬇️
scrapy/utils/log.py 88.34% <89.28%> (-1.24%) ⬇️
scrapy/utils/testproc.py 81.39% <90.47%> (+2.44%) ⬆️
scrapy/commands/__init__.py 74.71% <100.00%> (ø)
... and 15 more

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

I love it when adding type hints includes bug fixes :)

iterable = etree.iterparse(reader, tag=tag, encoding=reader.encoding)
# technically, etree.iterparse only needs .read() AFAICS, but this is how it's typed
iterable = etree.iterparse(
cast("SupportsReadClose[bytes]", reader), tag=tag, encoding=reader.encoding
Copy link
Member

Choose a reason for hiding this comment

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

💄 Would it work to move the type hint to line 60/86 above instead of casting here?

PS: I’m not sure we need the comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Casting hides reader.encoding.

@wRAR wRAR merged commit 85696d7 into scrapy:master Aug 9, 2023
@wRAR wRAR deleted the typing-utils-2 branch August 9, 2023 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants