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

PY3: use six.iterkeys, six.iteritems, and tempfile #799

Merged
merged 1 commit into from Jul 14, 2014

Conversation

@felixonmars
Copy link
Contributor

@felixonmars felixonmars commented Jul 14, 2014

This one is less trivial.

I was looking to change Headers.iteritems too, but that one seems different in behavior, so I left alone everything related to that.

@@ -77,7 +78,7 @@ def _format_cookie(self, cookie):
def _get_request_cookies(self, jar, request):
if isinstance(request.cookies, dict):
cookie_list = [{'name': k, 'value': v} for k, v in \
request.cookies.iteritems()]
six.iteritems(request.cookies)]

This comment has been minimized.

@kmike

kmike Jul 14, 2014
Member

IMHO there is no harm in using items() here. But iteritems is also fine :)

else:
field_iter = item.iterkeys()
field_iter = six.iterkeys(item)

This comment has been minimized.

@kmike

kmike Jul 14, 2014
Member

i guess it could be field_iter = item as well, but with iterkeys the intention is more clear - let's keep it

@@ -25,7 +26,7 @@ def tearDown(self):
rmtree(self.temp_path)

def call(self, *new_args, **kwargs):
out = os.tmpfile()
out = tempfile.TemporaryFile()

This comment has been minimized.

@kmike

kmike Jul 14, 2014
Member

I think it is better to either use context manager to close this file explicitly, or to use os.devnull.

This comment has been minimized.

@kmike

kmike Jul 14, 2014
Member

I'll take care of it.

This comment has been minimized.

@felixonmars

felixonmars Jul 14, 2014
Author Contributor

Thanks :)

@kmike
Copy link
Member

@kmike kmike commented Jul 14, 2014

This PR looks good.

Usually I try to avoid six.iteritems because it can be usually replaced with [(k, dct[v]) for k in dct] which should have less overhead. But now I actually measured that, and the speed difference is either non-existent or six.iteritems is faster. So six.iteritems is great.

kmike added a commit that referenced this pull request Jul 14, 2014
PY3: use six.iterkeys, six.iteritems, and tempfile
@kmike kmike merged commit 321a765 into scrapy:master Jul 14, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
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

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