This repository has been archived by the owner on Mar 20, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 13
Add support for Python 3 #120
Merged
Merged
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
5602e39
Add support for Python 3
geigerj 7a0cf38
Fix typo in tox.ini
geigerj 55a0aa1
Add requirements.txt as dep to docs environment
geigerj 282a409
Don't run Sphinx in Python 3 environments
geigerj c50ce93
Rename _iterator to _page_iterator
geigerj b32a42e
Turn off linting in Python 3 environments
geigerj 43e644c
Undo previous commit, disable travis caching
geigerj 79ae771
Upgrade grpcio to 1.0rc1
geigerj 63c86e4
Add linuxbrew bin directory to travis PATH
geigerj 114b8c1
Fix syntax in .travis.yml
geigerj 915b6f8
Switch to installing grpc with brew
geigerj 7a53816
Switch out methods deprecated in Python 3
geigerj 190b205
Upgrade pip, revert protobuf install changes
geigerj 5771c42
Disable linting in Python 3
geigerj dd59aa0
Re-enable pylint'ing, re-enable Travis caching
geigerj 1393e2a
Suppress pylint error
geigerj c359c98
Add pep8 env back to python3
geigerj dc2632c
Bump grpcio version in setup.py
geigerj 8d243bd
Add future library into setup.py
geigerj File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
future>=0.15.2 | ||
grpcio>=0.15.0 | ||
oauth2client>=1.5.2 | ||
ply==3.8 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change being made? It's a bit confusing, since it looks like it's calling
ResourceIterator.next()
, but I think it's really callingPageIterator.next()
.It would help to rename
self._iterator
toself._page_iterator
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Python 3 style is for iterators to have a special method
__next__
and then invoke the built-in functionnext()
to call it.The
next()
method is retained as an alias only for Python 2 compatibility -- in Python 3, the iterator's next method is not meant to be invoked directly.Renamed per your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Is this a canonical pattern for Python2/3 compatible code, or should it be explained more in the iterator
next()
docstrings?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect that it's the common way of doing, since there's really only one right way of implementing a custom iterator in each of Python 2 and 3, but I don't really know enough to be able to call it "canonical".
FWIW, this is also basically the way that is recommended by the writers of the
future
library, which is a well-known library for 2/3 intercompatibility: http://python-future.org/compatible_idioms.html#custom-iterators. (They don't show the aliasing of the oldnext()
method, but the presence of that method actually is necessary for Python 2 to recognize that this object is an iterator.)