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

Using dictionary comprehensions in place of dict(). #2386

Merged
merged 1 commit into from
Sep 22, 2016

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Sep 22, 2016

This is now possible after dropping support for Python 2.6.

Also tweaking a few places dict(...) was used to copy a dictionary to just use .copy().

We also have a lot of dict(parse_qsl(...)) going on, maybe we should use parse_qs and just deal with the slightly different output.

Also, we have

google/cloud/storage/batch.py:204:        return dict(multi._headers), body
google/cloud/storage/batch.py:319:        msg_headers = dict(sub_message._headers)

@tseaver I seem to recall a discussion as to why this wouldn't clobber any repeated headers, but would like some clarification.

This is now possible after dropping support for
Python 2.6.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 22, 2016
@tseaver
Copy link
Contributor

tseaver commented Sep 22, 2016

LGTM. dict(parse_qsl(query_sting)) returns single values, rather than lists, with the last one winning in case of duplicates. Given that we only use it in tests, where we know there aren't dupes, it is a simpler, cleaner form than the list of values returned by parse_qs(query_string).

@dhermes dhermes merged commit 9e23e1c into googleapis:master Sep 22, 2016
@dhermes dhermes deleted the use-dict-comprehensions branch September 22, 2016 19:10
@dhermes
Copy link
Contributor Author

dhermes commented Sep 22, 2016

@tseaver Indeed. It'd be nice if parse_qs had a flag to enforce uniqueness if desired

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants