From 408e81e7499efecb03465af8a77ed34241b43b5c Mon Sep 17 00:00:00 2001 From: Sourabh Bajaj Date: Fri, 14 Apr 2017 15:54:51 -0700 Subject: [PATCH] [BEAM-1964] Fix lint issues and pylint upgrade --- sdks/python/.pylintrc | 5 ++++- .../apache_beam/examples/cookbook/group_with_coder.py | 6 ++++-- sdks/python/apache_beam/internal/pickler.py | 8 ++++---- .../apache_beam/io/gcp/datastore/v1/datastoreio_test.py | 4 ++-- sdks/python/apache_beam/io/textio.py | 2 +- sdks/python/apache_beam/transforms/combiners.py | 4 ++-- sdks/python/apache_beam/transforms/core.py | 2 +- sdks/python/apache_beam/transforms/ptransform.py | 2 +- sdks/python/apache_beam/typehints/typehints_test.py | 2 +- sdks/python/apache_beam/utils/retry.py | 2 +- sdks/python/tox.ini | 2 +- 11 files changed, 22 insertions(+), 17 deletions(-) diff --git a/sdks/python/.pylintrc b/sdks/python/.pylintrc index 7a0611a4da83..e29e15b53a75 100644 --- a/sdks/python/.pylintrc +++ b/sdks/python/.pylintrc @@ -94,6 +94,7 @@ disable = import-error, import-self, invalid-name, + invalid-unary-operand-type, locally-disabled, locally-enabled, misplaced-bare-raise, @@ -104,6 +105,8 @@ disable = no-self-use, no-value-for-parameter, not-callable, + # Re-enable the context manager check once https://github.com/PyCQA/pylint/issues/782 is fixed + not-context-manager, pointless-statement, protected-access, raising-non-exception, @@ -115,6 +118,7 @@ disable = similarities, simplifiable-if-statement, super-init-not-called, + super-on-old-class, undefined-variable, unexpected-keyword-arg, unidiomatic-typecheck, @@ -124,7 +128,6 @@ disable = unused-wildcard-import, wildcard-import, - [REPORTS] # Tells whether to display a full report or only the messages reports=no diff --git a/sdks/python/apache_beam/examples/cookbook/group_with_coder.py b/sdks/python/apache_beam/examples/cookbook/group_with_coder.py index f6f210832f8f..cb675bdc6fac 100644 --- a/sdks/python/apache_beam/examples/cookbook/group_with_coder.py +++ b/sdks/python/apache_beam/examples/cookbook/group_with_coder.py @@ -78,9 +78,11 @@ def get_players(descriptor): return Player(name), int(points) -def run(argv=sys.argv[1:]): +def run(args=None): """Runs the workflow computing total points from a collection of matches.""" + if args is None: + args = sys.argv[1:] parser = argparse.ArgumentParser() parser.add_argument('--input', required=True, @@ -88,7 +90,7 @@ def run(argv=sys.argv[1:]): parser.add_argument('--output', required=True, help='Output file to write results to.') - known_args, pipeline_args = parser.parse_known_args(argv) + known_args, pipeline_args = parser.parse_known_args(args) # We use the save_main_session option because one or more DoFn's in this # workflow rely on global context (e.g., a module imported at module level). pipeline_options = PipelineOptions(pipeline_args) diff --git a/sdks/python/apache_beam/internal/pickler.py b/sdks/python/apache_beam/internal/pickler.py index a4ab7b9c97ee..3f3f65761d6a 100644 --- a/sdks/python/apache_beam/internal/pickler.py +++ b/sdks/python/apache_beam/internal/pickler.py @@ -184,12 +184,12 @@ def new_log_info(msg, *args, **kwargs): def dumps(o, enable_trace=True): try: s = dill.dumps(o) - except Exception as e: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except if enable_trace: dill.dill._trace(True) # pylint: disable=protected-access s = dill.dumps(o) else: - raise e + raise finally: dill.dill._trace(False) # pylint: disable=protected-access @@ -210,12 +210,12 @@ def loads(encoded, enable_trace=True): try: return dill.loads(s) - except Exception as e: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except if enable_trace: dill.dill._trace(True) # pylint: disable=protected-access return dill.loads(s) else: - raise e + raise finally: dill.dill._trace(False) # pylint: disable=protected-access diff --git a/sdks/python/apache_beam/io/gcp/datastore/v1/datastoreio_test.py b/sdks/python/apache_beam/io/gcp/datastore/v1/datastoreio_test.py index 3121d3a72743..8eed0f83153b 100644 --- a/sdks/python/apache_beam/io/gcp/datastore/v1/datastoreio_test.py +++ b/sdks/python/apache_beam/io/gcp/datastore/v1/datastoreio_test.py @@ -27,7 +27,7 @@ from apache_beam.io.gcp.datastore.v1.datastoreio import WriteToDatastore # Protect against environments where datastore library is not available. -# pylint: disable=wrong-import-order, wrong-import-position +# pylint: disable=wrong-import-order, wrong-import-position, ungrouped-imports try: from google.cloud.proto.datastore.v1 import datastore_pb2 from google.cloud.proto.datastore.v1 import query_pb2 @@ -35,7 +35,7 @@ from googledatastore import helper as datastore_helper except ImportError: datastore_pb2 = None -# pylint: enable=wrong-import-order, wrong-import-position +# pylint: enable=wrong-import-order, wrong-import-position, ungrouped-imports @unittest.skipIf(datastore_pb2 is None, 'GCP dependencies are not installed') diff --git a/sdks/python/apache_beam/io/textio.py b/sdks/python/apache_beam/io/textio.py index b6a24b09e157..f2c3d34aaef5 100644 --- a/sdks/python/apache_beam/io/textio.py +++ b/sdks/python/apache_beam/io/textio.py @@ -160,7 +160,7 @@ def split_points_unclaimed(stop_position): # followed by a new line character. Since such a record is at the last # position of a file, it should not be a part of the considered range. # We do this check to ignore such records. - if len(record) == 0 and num_bytes_to_next_record < 0: + if len(record) == 0 and num_bytes_to_next_record < 0: # pylint: disable=len-as-condition break # Record separator must be larger than zero bytes. diff --git a/sdks/python/apache_beam/transforms/combiners.py b/sdks/python/apache_beam/transforms/combiners.py index f81283257638..fa0742db4179 100644 --- a/sdks/python/apache_beam/transforms/combiners.py +++ b/sdks/python/apache_beam/transforms/combiners.py @@ -463,7 +463,7 @@ def add_input(self, accumulator, element): class ToList(ptransform.PTransform): """A global CombineFn that condenses a PCollection into a single list.""" - def __init__(self, label='ToList'): + def __init__(self, label='ToList'): # pylint: disable=useless-super-delegation super(ToList, self).__init__(label) def expand(self, pcoll): @@ -497,7 +497,7 @@ class ToDict(ptransform.PTransform): will be present in the resulting dict. """ - def __init__(self, label='ToDict'): + def __init__(self, label='ToDict'): # pylint: disable=useless-super-delegation super(ToDict, self).__init__(label) def expand(self, pcoll): diff --git a/sdks/python/apache_beam/transforms/core.py b/sdks/python/apache_beam/transforms/core.py index b1a33eaee426..3def9efe235a 100644 --- a/sdks/python/apache_beam/transforms/core.py +++ b/sdks/python/apache_beam/transforms/core.py @@ -1172,7 +1172,7 @@ class Windowing(object): def __init__(self, windowfn, triggerfn=None, accumulation_mode=None, output_time_fn=None): - global AccumulationMode, DefaultTrigger + global AccumulationMode, DefaultTrigger # pylint: disable=global-variable-not-assigned # pylint: disable=wrong-import-order, wrong-import-position from apache_beam.transforms.trigger import AccumulationMode, DefaultTrigger # pylint: enable=wrong-import-order, wrong-import-position diff --git a/sdks/python/apache_beam/transforms/ptransform.py b/sdks/python/apache_beam/transforms/ptransform.py index 0ac8b5b45b92..9b7a37ffe047 100644 --- a/sdks/python/apache_beam/transforms/ptransform.py +++ b/sdks/python/apache_beam/transforms/ptransform.py @@ -656,7 +656,7 @@ def __init__(self, transform, label): super(_NamedPTransform, self).__init__(label) self.transform = transform - def __ror__(self, pvalueish): + def __ror__(self, pvalueish, _unused=None): return self.transform.__ror__(pvalueish, self.label) def expand(self, pvalue): diff --git a/sdks/python/apache_beam/typehints/typehints_test.py b/sdks/python/apache_beam/typehints/typehints_test.py index 4e82fbcd5fe5..8ebe3e493e8b 100644 --- a/sdks/python/apache_beam/typehints/typehints_test.py +++ b/sdks/python/apache_beam/typehints/typehints_test.py @@ -49,7 +49,7 @@ def check_type_hints(f): @functools.wraps(f) def wrapper(*args, **kwargs): hints = get_type_hints(f) - if hints.input_types: + if hints.input_types: # pylint: disable=too-many-nested-blocks input_hints = getcallargs_forhints( f, *hints.input_types[0], **hints.input_types[1]) inputs = inspect.getcallargs(f, *args, **kwargs) diff --git a/sdks/python/apache_beam/utils/retry.py b/sdks/python/apache_beam/utils/retry.py index 4b137e2c8902..2c32f0f775d1 100644 --- a/sdks/python/apache_beam/utils/retry.py +++ b/sdks/python/apache_beam/utils/retry.py @@ -176,7 +176,7 @@ def wrapper(*args, **kwargs): sleep_interval = retry_intervals.next() except StopIteration: # Re-raise the original exception since we finished the retries. - raise exn, None, exn_traceback + raise exn, None, exn_traceback # pylint: disable=raising-bad-type logger( 'Retry with exponential backoff: waiting for %s seconds before ' diff --git a/sdks/python/tox.ini b/sdks/python/tox.ini index 63e197de586c..6660919404da 100644 --- a/sdks/python/tox.ini +++ b/sdks/python/tox.ini @@ -73,7 +73,7 @@ passenv = TRAVIS* deps= nose==1.3.7 pep8==1.7.0 - pylint==1.6.5 + pylint==1.7.0 commands = pip install -e .[test] {toxinidir}/run_pylint.sh