From aefdd225eb639ec933a811b42711e95df6aed3c6 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Fri, 22 Apr 2016 09:36:11 -0700 Subject: [PATCH] Re-enabling redefined-builtin for Pylint. In the process, refactoring places where builtins were used intentionally or un-intentionally. --- gcloud/bigtable/happybase/table.py | 31 ++++++++++++++++------------- gcloud/datastore/test_batch.py | 8 ++++---- gcloud/datastore/test_connection.py | 18 ++++++++--------- gcloud/exceptions.py | 2 +- gcloud/search/document.py | 5 +++-- scripts/pylintrc_default | 3 --- system_tests/clear_datastore.py | 4 ++-- system_tests/populate_datastore.py | 4 ++-- 8 files changed, 38 insertions(+), 37 deletions(-) diff --git a/gcloud/bigtable/happybase/table.py b/gcloud/bigtable/happybase/table.py index bf018ad1647f..3f87f953c026 100644 --- a/gcloud/bigtable/happybase/table.py +++ b/gcloud/bigtable/happybase/table.py @@ -296,7 +296,7 @@ def cells(self, row, column, versions=None, timestamp=None, curr_cells, include_timestamp=include_timestamp) def scan(self, row_start=None, row_stop=None, row_prefix=None, - columns=None, filter=None, timestamp=None, + columns=None, timestamp=None, include_timestamp=False, limit=None, **kwargs): """Create a scanner for data in this table. @@ -314,6 +314,15 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None, omitted, a full table scan is done. Note that this usually results in severe performance problems. + The keyword argument ``filter`` is also supported (beyond column and + row range filters supported here). HappyBase / HBase users will have + used this as an HBase filter string. (See the `Thrift docs`_ for more + details on those filters.) However, Google Cloud Bigtable doesn't + support those filter strings so a + :class:`~gcloud.bigtable.row.RowFilter` should be used instead. + + .. _Thrift docs: http://hbase.apache.org/0.94/book/thrift.html + The arguments ``batch_size``, ``scan_batching`` and ``sorted_columns`` are allowed (as keyword arguments) for compatibility with HappyBase. However, they will not be used in any way, and will cause a @@ -348,13 +357,6 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None, * an entire column family: ``fam`` or ``fam:`` * a single column: ``fam:col`` - :type filter: :class:`RowFilter ` - :param filter: (Optional) An additional filter (beyond column and - row range filters supported here). HappyBase / HBase - users will have used this as an HBase filter string. See - http://hbase.apache.org/0.94/book/thrift.html - for more details on those filters. - :type timestamp: int :param timestamp: (Optional) Timestamp (in milliseconds since the epoch). If specified, only cells returned before (or @@ -376,6 +378,7 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None, :class:`TypeError ` if a string ``filter`` is used. """ + filter_ = kwargs.pop('filter', None) legacy_args = [] for kw_name in ('batch_size', 'scan_batching', 'sorted_columns'): if kw_name in kwargs: @@ -399,22 +402,22 @@ def scan(self, row_start=None, row_stop=None, row_prefix=None, row_stop = _string_successor(row_prefix) filters = [] - if isinstance(filter, six.string_types): + if isinstance(filter_, six.string_types): raise TypeError('Specifying filters as a string is not supported ' 'by Cloud Bigtable. Use a ' 'gcloud.bigtable.row.RowFilter instead.') - elif filter is not None: - filters.append(filter) + elif filter_ is not None: + filters.append(filter_) if columns is not None: filters.append(_columns_filter_helper(columns)) # versions == 1 since we only want the latest. - filter_ = _filter_chain_helper(versions=1, timestamp=timestamp, - filters=filters) + filter_chain = _filter_chain_helper(versions=1, timestamp=timestamp, + filters=filters) partial_rows_data = self._low_level_table.read_rows( start_key=row_start, end_key=row_stop, - limit=limit, filter_=filter_) + limit=limit, filter_=filter_chain) # Mutable copy of data. rows_dict = partial_rows_data.rows diff --git a/gcloud/datastore/test_batch.py b/gcloud/datastore/test_batch.py index 4636f275979f..8c267dee5344 100644 --- a/gcloud/datastore/test_batch.py +++ b/gcloud/datastore/test_batch.py @@ -298,14 +298,14 @@ def test_as_context_mgr_w_error(self): class _PathElementPB(object): - def __init__(self, id): - self.id = id + def __init__(self, id_): + self.id = id_ class _KeyPB(object): - def __init__(self, id): - self.path = [_PathElementPB(id)] + def __init__(self, id_): + self.path = [_PathElementPB(id_)] class _Connection(object): diff --git a/gcloud/datastore/test_connection.py b/gcloud/datastore/test_connection.py index f7a036bd4796..513c0cbaa251 100644 --- a/gcloud/datastore/test_connection.py +++ b/gcloud/datastore/test_connection.py @@ -22,11 +22,11 @@ def _getTargetClass(self): return Connection - def _make_key_pb(self, project, id=1234): + def _make_key_pb(self, project, id_=1234): from gcloud.datastore.key import Key path_args = ('Kind',) - if id is not None: - path_args += (id,) + if id_ is not None: + path_args += (id_,) return Key(*path_args, project=project).to_protobuf() def _make_query_pb(self, kind): @@ -362,7 +362,7 @@ def test_lookup_multiple_keys_empty_response(self): PROJECT = 'PROJECT' key_pb1 = self._make_key_pb(PROJECT) - key_pb2 = self._make_key_pb(PROJECT, id=2345) + key_pb2 = self._make_key_pb(PROJECT, id_=2345) rsp_pb = datastore_pb2.LookupResponse() conn = self._makeOne() URI = '/'.join([ @@ -391,7 +391,7 @@ def test_lookup_multiple_keys_w_missing(self): PROJECT = 'PROJECT' key_pb1 = self._make_key_pb(PROJECT) - key_pb2 = self._make_key_pb(PROJECT, id=2345) + key_pb2 = self._make_key_pb(PROJECT, id_=2345) rsp_pb = datastore_pb2.LookupResponse() er_1 = rsp_pb.missing.add() er_1.entity.key.CopyFrom(key_pb1) @@ -425,7 +425,7 @@ def test_lookup_multiple_keys_w_deferred(self): PROJECT = 'PROJECT' key_pb1 = self._make_key_pb(PROJECT) - key_pb2 = self._make_key_pb(PROJECT, id=2345) + key_pb2 = self._make_key_pb(PROJECT, id_=2345) rsp_pb = datastore_pb2.LookupResponse() rsp_pb.deferred.add().CopyFrom(key_pb1) rsp_pb.deferred.add().CopyFrom(key_pb2) @@ -778,12 +778,12 @@ def test_allocate_ids_non_empty(self): PROJECT = 'PROJECT' before_key_pbs = [ - self._make_key_pb(PROJECT, id=None), - self._make_key_pb(PROJECT, id=None), + self._make_key_pb(PROJECT, id_=None), + self._make_key_pb(PROJECT, id_=None), ] after_key_pbs = [ self._make_key_pb(PROJECT), - self._make_key_pb(PROJECT, id=2345), + self._make_key_pb(PROJECT, id_=2345), ] rsp_pb = datastore_pb2.AllocateIdsResponse() rsp_pb.keys.add().CopyFrom(after_key_pbs[0]) diff --git a/gcloud/exceptions.py b/gcloud/exceptions.py index 49f16e00f2e8..fffdb1f5c477 100644 --- a/gcloud/exceptions.py +++ b/gcloud/exceptions.py @@ -147,7 +147,7 @@ class InternalServerError(ServerError): code = 500 -class NotImplemented(ServerError): +class MethodNotImplemented(ServerError): """Exception mapping a '501 Not Implemented' response.""" code = 501 diff --git a/gcloud/search/document.py b/gcloud/search/document.py index ecbff93ba16e..1fa645e953dc 100644 --- a/gcloud/search/document.py +++ b/gcloud/search/document.py @@ -204,8 +204,9 @@ def _parse_value_resource(resource): return TimestampValue(value) if 'geoValue' in resource: lat_long = resource['geoValue'] - lat, long = [float(coord.strip()) for coord in lat_long.split(',')] - return GeoValue((lat, long)) + latitude, longitude = [float(coord.strip()) + for coord in lat_long.split(',')] + return GeoValue((latitude, longitude)) raise ValueError("Unknown value type") def _parse_fields_resource(self, resource): diff --git a/scripts/pylintrc_default b/scripts/pylintrc_default index b4d01032619d..df421c57f6b4 100644 --- a/scripts/pylintrc_default +++ b/scripts/pylintrc_default @@ -68,8 +68,6 @@ load-plugins=pylint.extensions.check_docs # - maybe-no-member: bi-modal functions confuse pylint type inference. # - no-member: indirections in protobuf-generated code # - protected-access: helpers use '_foo' of classes from generated code. -# - redefined-builtin: use of 'id', 'type', 'filter' args in API-bound funcs; -# use of 'NotImplemented' to map HTTP response code. # - similarities: 'Bucket' and 'Blob' define 'metageneration' and 'owner' with # identical implementation but different docstrings. # - star-args: standard Python idioms for varargs: @@ -93,7 +91,6 @@ disable = maybe-no-member, no-member, protected-access, - redefined-builtin, similarities, star-args, redefined-variable-type, diff --git a/system_tests/clear_datastore.py b/system_tests/clear_datastore.py index 96d214bc464d..922802d32af2 100644 --- a/system_tests/clear_datastore.py +++ b/system_tests/clear_datastore.py @@ -18,7 +18,7 @@ import os -from six.moves import input +import six from gcloud import datastore from gcloud.environment_vars import TESTS_PROJECT @@ -99,7 +99,7 @@ def remove_all_entities(client=None): print_func('This command will remove all entities for ' 'the following kinds:') print_func('\n'.join(['- ' + val for val in ALL_KINDS])) - response = input('Is this OK [y/n]? ') + response = six.moves.input('Is this OK [y/n]? ') if response.lower() == 'y': remove_all_entities() else: diff --git a/system_tests/populate_datastore.py b/system_tests/populate_datastore.py index 041471bd0a0c..6a6679ff521a 100644 --- a/system_tests/populate_datastore.py +++ b/system_tests/populate_datastore.py @@ -19,7 +19,7 @@ import os -from six.moves import zip +import six from gcloud import datastore from gcloud.environment_vars import TESTS_PROJECT @@ -93,7 +93,7 @@ def add_characters(client=None): # Get a client that uses the test dataset. client = datastore.Client(project=os.getenv(TESTS_PROJECT)) with client.transaction() as xact: - for key_path, character in zip(KEY_PATHS, CHARACTERS): + for key_path, character in six.moves.zip(KEY_PATHS, CHARACTERS): if key_path[-1] != character['name']: raise ValueError(('Character and key don\'t agree', key_path, character))