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

[MRG+1] Revise/modernize item exporter example in docs #2989

Merged
merged 2 commits into from Nov 3, 2017

Conversation

@colinmorris
Copy link
Contributor

@colinmorris colinmorris commented Oct 31, 2017

Addresses #2932

@kmike pointed out that the concept of a file-per-spider no longer made sense, so I changed the use case to one inspired by my own experience. It's maybe a little more complicated than the previous example, but it still comes out to fewer lines of code after removing the overhead of manually connecting the signals.

@codecov
Copy link

@codecov codecov bot commented Oct 31, 2017

Codecov Report

Merging #2989 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2989      +/-   ##
==========================================
+ Coverage   84.58%   84.59%   +0.01%     
==========================================
  Files         164      164              
  Lines        9249     9249              
  Branches     1376     1376              
==========================================
+ Hits         7823     7824       +1     
  Misses       1167     1167              
+ Partials      259      258       -1
Impacted Files Coverage Δ
scrapy/utils/trackref.py 86.48% <0%> (+2.7%) ⬆️
self.year_to_exporter = {}

def close_spider(self, spider):
for exporter in self.year_to_exporter.itervalues():

This comment has been minimized.

@kmike

kmike Oct 31, 2017
Member

It should be .values() in order to be compatible with Python 3. Creating a temporary list in Python 2 is not a problem as a list would be small.

def _exporter_for_item(self, item):
year = item['year']
if year not in self.year_to_exporter:
f = open('{}.xml'.format(year), 'w+b')

This comment has been minimized.

@kmike

kmike Oct 31, 2017
Member

why is 'w+b' needed, not just 'wb'?

This comment has been minimized.

@colinmorris

colinmorris Oct 31, 2017
Author Contributor

Good call. I just copied it from the previous example, but as you say, it doesn't seem to be necessary.

@kmike
Copy link
Member

@kmike kmike commented Oct 31, 2017

I like the revised example, exporting to several locations is a common problem users ask for 👍
The PR looks good to me, besides a couple of minor comments.

@kmike kmike changed the title Revise/modernize item exporter example in docs [MRG+1] Revise/modernize item exporter example in docs Nov 2, 2017
@kmike
Copy link
Member

@kmike kmike commented Nov 2, 2017

Looks good to me, thanks @colinmorris!

@kmike kmike added the docs label Nov 2, 2017
@dangra dangra merged commit b8870ee into scrapy:master Nov 3, 2017
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
codecov/patch Coverage not affected when comparing 108f8c4...23e571e
Details
@kmike kmike added this to the v1.5 milestone Dec 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.