Skip to content
This repository has been archived by the owner on Mar 20, 2018. It is now read-only.

Add support for Python 3 #120

Merged
merged 19 commits into from
Aug 5, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ cache:
language: python
python:
- "2.7"
- "3.4"
- "3.5"
install: pip install codecov tox-travis
script: tox
after_success:
Expand Down
10 changes: 9 additions & 1 deletion google/gax/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,10 @@ def __iter__(self):
return self

def next(self):
"""For Python 2.7 compatibility; see __next__."""
return self.__next__()

def __next__(self):
"""Retrieves the next page."""
if self._done:
raise StopIteration
Expand Down Expand Up @@ -464,10 +468,14 @@ def __iter__(self):
return self

def next(self):
"""For Python 2.7 compatibility; see __next__."""
return self.__next__()

def __next__(self):
"""Retrieves the next resource."""
# pylint: disable=next-method-called
while not self._current:
self._current = self._iterator.next()
self._current = next(self._iterator)
Copy link
Contributor

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 calling PageIterator.next().

It would help to rename self._iterator to self._page_iterator.

Copy link
Contributor Author

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 function next() 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.

Copy link
Contributor

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?

Copy link
Contributor Author

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 old next() method, but the presence of that method actually is necessary for Python 2 to recognize that this object is an iterator.)

self._index = 0
resource = self._current[self._index]
self._index += 1
Expand Down
6 changes: 3 additions & 3 deletions google/gax/api_callable.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@

from __future__ import absolute_import, division
import random
import sys
import time

from future.utils import raise_with_traceback

from . import (BackoffSettings, BundleOptions, bundling, CallSettings, config,
PageIterator, ResourceIterator, RetryOptions)
from .errors import GaxError, RetryError
Expand Down Expand Up @@ -429,8 +430,7 @@ def inner(*args, **kwargs):
return a_func(*args, **kwargs)
# pylint: disable=catching-non-exception
except tuple(errors) as exception:
raise (GaxError('RPC failed', cause=exception), None,
sys.exc_info()[2])
raise_with_traceback(GaxError('RPC failed', cause=exception))

return inner

Expand Down
1 change: 1 addition & 0 deletions requirements.txt
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
Expand Down
8 changes: 5 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
[tox]
envlist = py27,pep8,pylint-errors,pylint-full
envlist = py27,py34,py35,pep8,pylint-errors,pylint-full

[tox:travis]
2.7 = py27, pep8, pylint-full, docs
2.7 = py27,pep8,pylint-full,docs
3.4 = py34,pep8,pylint-full
3.5 = py35,pep8,pylint-full

[testenv]
setenv =
PYTHONPATH = {toxinidir}:{toxinidir}/src-gen/test

deps = -r{toxinidir}/test-requirements.txt
-r{toxinidir}/requirements.txt
whitelist_externals = mkdir
Expand Down Expand Up @@ -55,5 +56,6 @@ commands =
sphinx-build -W -b html -d docs/_build/doctrees docs docs/_build/html
sphinx-build -b latex -D language=en -d _build/doctrees docs _build/latex
deps =
-r{toxinidir}/requirements.txt
Sphinx
sphinx_rtd_theme