Skip to content

Update ItemFilter #5203

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 1 commit into from
Jul 14, 2021
Merged

Update ItemFilter #5203

merged 1 commit into from
Jul 14, 2021

Conversation

elacuesta
Copy link
Member

@elacuesta elacuesta commented Jul 14, 2021

Some updates to the ItemFilter class introduced in #5178. Sorry, I was only able to take a look after it was merged.

  • Avoid the (perhaps negligible) overhead of converting from set to tuple on each accepts invocation.
  • Add types. Ideally, item should be annotated as "anything that returns True for itemadapter.ItemAdapter.is_item". I'm not sure how to do that at the moment, suggestions are welcome. (I've looked at Protocols for duck typing hints, but that doesn't seem to solve this particular boolean predicate problem.)
  • Check that feed_options is not None. I'm not sure this is actually a problem, but many classes define feed_options=None as default keyword argument, so I suspect there might be a case in which this argument might be passed as None and trying to access it like a dict will result in a TypeError. I'll try to do some more research to make sure this is a real problem (or remove the check and simplify the implementation if it's not).

@codecov
Copy link

codecov bot commented Jul 14, 2021

Codecov Report

Merging #5203 (cf4ae4f) into master (d7deba7) will decrease coverage by 0.03%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #5203      +/-   ##
==========================================
- Coverage   84.16%   84.13%   -0.04%     
==========================================
  Files         162      162              
  Lines       10524    10526       +2     
  Branches     1522     1522              
==========================================
- Hits         8858     8856       -2     
- Misses       1406     1408       +2     
- Partials      260      262       +2     
Impacted Files Coverage Δ
scrapy/extensions/feedexport.py 91.05% <80.00%> (-0.61%) ⬇️
scrapy/core/downloader/__init__.py 90.97% <0.00%> (-1.51%) ⬇️

@Gallaecio Gallaecio merged commit bcce066 into scrapy:master Jul 14, 2021
@elacuesta elacuesta deleted the item-filter-types branch July 14, 2021 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants