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

Allow spiders to also return/yield ItemLoader. #3244

Closed
wants to merge 2 commits into from
Closed

Allow spiders to also return/yield ItemLoader. #3244

wants to merge 2 commits into from

Conversation

vincent-ferotin
Copy link

Behind, Scrapy will convert ItemLoader to Item by calling
:meth:ItemLoader.load_item.

Behind, Scrapy will convert ItemLoader to Item by calling
:meth:`ItemLoader.load_item`.
@whalebot-helmsman
Copy link
Contributor

@vincent-ferotin Problem with unit-tests was fixed in master. Can you merge master to this PR?

@codecov
Copy link

codecov bot commented May 18, 2018

Codecov Report

Merging #3244 into master will decrease coverage by 0.01%.
The diff coverage is 25%.

@@            Coverage Diff            @@
##           master   #3244      +/-   ##
=========================================
- Coverage   82.12%   82.1%   -0.02%     
=========================================
  Files         228     228              
  Lines        9599    9602       +3     
  Branches     1385    1386       +1     
=========================================
+ Hits         7883    7884       +1     
- Misses       1457    1458       +1     
- Partials      259     260       +1
Impacted Files Coverage Δ
scrapy/core/scraper.py 85.33% <25%> (-1.07%) ⬇️

@vincent-ferotin
Copy link
Author

@whalebot-helmsman Here it is, I hope so? (not enough familiar/confident with git/GitHub to be sure, please let tell me if it is not ok for you ^^;)

@whalebot-helmsman
Copy link
Contributor

Thanks, @vincent-ferotin mentioned problems with twisted is gone. I don't know the reason why other tests are in failed state.

@kmike
Copy link
Member

kmike commented Jun 22, 2018

Hi @vincent-ferotin! I can see how this feature can save some typing, and API improvements are good to have. However, currently I'm -0 to add such feature because:

  • there was an idea to move ItemLoaders out of Scrapy;
  • it complicates API of a spider; custom middlewares would need to be modified in order to support a new possible output type (?).

The implementation also needs work:

@Gallaecio
Copy link
Member

Closing as, even if implemented, https://github.com/scrapy/itemadapter would be the right place now.

@Gallaecio Gallaecio closed this Apr 11, 2023
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.

None yet

4 participants