You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by pa...@apache.org on 2020/05/02 21:40:49 UTC

[beam] branch master updated: Merge pull request #11523 from [BEAM-8414] Cleanup Python codebase to enable some of the excluded Python lint checks.

This is an automated email from the ASF dual-hosted git repository.

pabloem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git


The following commit(s) were added to refs/heads/master by this push:
     new 360de38  Merge pull request #11523 from [BEAM-8414] Cleanup Python codebase to enable some of the excluded Python lint checks.
360de38 is described below

commit 360de38e5047f4ab26b1dbec5c7c6233fd33b20b
Author: Stephen O'Kennedy <st...@gmail.com>
AuthorDate: Sat May 2 23:40:25 2020 +0200

    Merge pull request #11523 from [BEAM-8414] Cleanup Python codebase to enable some of the excluded Python lint checks.
    
    * [BEAM-8414] Cleanup Python codebase to enable some of the excluded
    Python lint checks.
    
    * Renabled the following pylint checks:
      * consider-using-set-comprehension
      * chained-comparison
      * consider-using-sys-exit
    * Cleaned up the codebase to satisfy the enabled checks
    
    * Update sdks/python/apache_beam/io/restriction_trackers.py
    
    Co-Authored-By: Pablo <pa...@users.noreply.github.com>
    
    * BEAM-8414
    * Fixed linting and formatter errors
    
    * BEAM-8414
    
    * Reverted `consider-using-sys-ext` linter change
    
    * BEAM-8418
    
    * Reran the py3-yapf to reformat existing files
    
    * Removed unused sys import
    
    Co-authored-by: Pablo <pa...@users.noreply.github.com>
    Co-authored-by: Stephen O'Kennedy <st...@ikea.com>
---
 sdks/python/.pylintrc                              |  2 -
 sdks/python/apache_beam/io/fileio_test.py          |  4 +-
 .../apache_beam/io/gcp/bigquery_file_loads_test.py |  3 +-
 sdks/python/apache_beam/io/gcp/gcsio_test.py       |  2 +-
 sdks/python/apache_beam/io/restriction_trackers.py |  2 +-
 sdks/python/apache_beam/metrics/execution_test.py  |  6 ++-
 sdks/python/apache_beam/pipeline.py                |  8 ++--
 sdks/python/apache_beam/pipeline_test.py           | 54 +++++++++++-----------
 .../runners/interactive/interactive_beam.py        |  2 +-
 .../portability/fn_api_runner/translations.py      |  3 +-
 .../apache_beam/runners/worker/opcounters_test.py  |  2 +-
 11 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/sdks/python/.pylintrc b/sdks/python/.pylintrc
index 8e46902..1f92812 100644
--- a/sdks/python/.pylintrc
+++ b/sdks/python/.pylintrc
@@ -86,11 +86,9 @@ disable =
   bad-super-call,
   bad-continuation,
   broad-except,
-  chained-comparison,
   comparison-with-callable,
   consider-using-enumerate,
   consider-using-in,
-  consider-using-set-comprehension,
   consider-using-sys-exit,
   cyclic-import,
   design,
diff --git a/sdks/python/apache_beam/io/fileio_test.py b/sdks/python/apache_beam/io/fileio_test.py
index 37e2d5b..992c635 100644
--- a/sdks/python/apache_beam/io/fileio_test.py
+++ b/sdks/python/apache_beam/io/fileio_test.py
@@ -357,8 +357,8 @@ class WriteFilesTest(_TestCaseWithTempDirCleanUp):
 
   CSV_HEADERS = ['project', 'foundation']
 
-  SIMPLE_COLLECTION_VALIDATION_SET = set([(elm['project'], elm['foundation'])
-                                          for elm in SIMPLE_COLLECTION])
+  SIMPLE_COLLECTION_VALIDATION_SET = {(elm['project'], elm['foundation'])
+                                      for elm in SIMPLE_COLLECTION}
 
   class CsvSink(fileio.TextSink):
     def __init__(self, headers):
diff --git a/sdks/python/apache_beam/io/gcp/bigquery_file_loads_test.py b/sdks/python/apache_beam/io/gcp/bigquery_file_loads_test.py
index 83bbf26..f9e0212 100644
--- a/sdks/python/apache_beam/io/gcp/bigquery_file_loads_test.py
+++ b/sdks/python/apache_beam/io/gcp/bigquery_file_loads_test.py
@@ -104,8 +104,7 @@ _DESTINATION_ELEMENT_PAIRS = [
     }),
 ]
 
-_DISTINCT_DESTINATIONS = list(
-    set([elm[0] for elm in _DESTINATION_ELEMENT_PAIRS]))
+_DISTINCT_DESTINATIONS = list({elm[0] for elm in _DESTINATION_ELEMENT_PAIRS})
 
 _ELEMENTS = [elm[1] for elm in _DESTINATION_ELEMENT_PAIRS]
 
diff --git a/sdks/python/apache_beam/io/gcp/gcsio_test.py b/sdks/python/apache_beam/io/gcp/gcsio_test.py
index 17fcf53..b01150c 100644
--- a/sdks/python/apache_beam/io/gcp/gcsio_test.py
+++ b/sdks/python/apache_beam/io/gcp/gcsio_test.py
@@ -117,7 +117,7 @@ class FakeGcsObjects(object):
       stream = download.stream
 
       def get_range_callback(start, end):
-        if not (start >= 0 and end >= start and end < len(f.contents)):
+        if not 0 <= start <= end < len(f.contents):
           raise ValueError(
               'start=%d end=%d len=%s' % (start, end, len(f.contents)))
         stream.write(f.contents[start:end + 1])
diff --git a/sdks/python/apache_beam/io/restriction_trackers.py b/sdks/python/apache_beam/io/restriction_trackers.py
index 29788d9..c623f38 100644
--- a/sdks/python/apache_beam/io/restriction_trackers.py
+++ b/sdks/python/apache_beam/io/restriction_trackers.py
@@ -137,7 +137,7 @@ class OffsetRestrictionTracker(RestrictionTracker):
           'of the range. Tried to claim position %r for the range [%r, %r)' %
           (position, self._range.start, self._range.stop))
 
-    if position >= self._range.start and position < self._range.stop:
+    if self._range.start <= position < self._range.stop:
       self._current_position = position
       return True
 
diff --git a/sdks/python/apache_beam/metrics/execution_test.py b/sdks/python/apache_beam/metrics/execution_test.py
index a91eeca..d1086da 100644
--- a/sdks/python/apache_beam/metrics/execution_test.py
+++ b/sdks/python/apache_beam/metrics/execution_test.py
@@ -104,9 +104,11 @@ class TestMetricsContainer(unittest.TestCase):
     self.assertEqual(len(cumulative.gauges), 10)
 
     self.assertEqual(
-        set(all_values), set([v for _, v in cumulative.counters.items()]))
+        set(all_values), {v
+                          for _, v in cumulative.counters.items()})
     self.assertEqual(
-        set(all_values), set([v.value for _, v in cumulative.gauges.items()]))
+        set(all_values), {v.value
+                          for _, v in cumulative.gauges.items()})
 
 
 if __name__ == '__main__':
diff --git a/sdks/python/apache_beam/pipeline.py b/sdks/python/apache_beam/pipeline.py
index 1fb0508..e14045e 100644
--- a/sdks/python/apache_beam/pipeline.py
+++ b/sdks/python/apache_beam/pipeline.py
@@ -121,7 +121,7 @@ class Pipeline(object):
   All the transforms applied to the pipeline must have distinct full labels.
   If same transform instance needs to be applied then the right shift operator
   should be used to designate new names
-  (e.g. ``input | "label" >> my_tranform``).
+  (e.g. ``input | "label" >> my_transform``).
   """
 
   # TODO: BEAM-9001 - set environment ID in all transforms and allow runners to
@@ -878,8 +878,10 @@ class Pipeline(object):
     root_transform_id, = proto.root_transform_ids
     p.transforms_stack = [context.transforms.get_by_id(root_transform_id)]
     # TODO(robertwb): These are only needed to continue construction. Omit?
-    p.applied_labels = set(
-        [t.unique_name for t in proto.components.transforms.values()])
+    p.applied_labels = {
+        t.unique_name
+        for t in proto.components.transforms.values()
+    }
     for id in proto.components.pcollections:
       pcollection = context.pcollections.get_by_id(id)
       pcollection.pipeline = p
diff --git a/sdks/python/apache_beam/pipeline_test.py b/sdks/python/apache_beam/pipeline_test.py
index f970758..0011109 100644
--- a/sdks/python/apache_beam/pipeline_test.py
+++ b/sdks/python/apache_beam/pipeline_test.py
@@ -262,8 +262,8 @@ class PipelineTest(unittest.TestCase):
 
     visitor = PipelineTest.Visitor(visited=[])
     pipeline.visit(visitor)
-    self.assertEqual(
-        set([pcoll1, pcoll2, pcoll3, pcoll4, pcoll5]), set(visitor.visited))
+    self.assertEqual({pcoll1, pcoll2, pcoll3, pcoll4, pcoll5},
+                     set(visitor.visited))
     self.assertEqual(set(visitor.enter_composite), set(visitor.leave_composite))
     self.assertEqual(2, len(visitor.enter_composite))
     self.assertEqual(visitor.enter_composite[1].transform, transform)
@@ -779,31 +779,31 @@ class PipelineOptionsTest(unittest.TestCase):
 
   def test_dir(self):
     options = Breakfast()
-    self.assertEqual(
-        set([
-            'from_dictionary',
-            'get_all_options',
-            'slices',
-            'style',
-            'view_as',
-            'display_data'
-        ]),
-        set([
-            attr for attr in dir(options)
-            if not attr.startswith('_') and attr != 'next'
-        ]))
-    self.assertEqual(
-        set([
-            'from_dictionary',
-            'get_all_options',
-            'style',
-            'view_as',
-            'display_data'
-        ]),
-        set([
-            attr for attr in dir(options.view_as(Eggs))
-            if not attr.startswith('_') and attr != 'next'
-        ]))
+    self.assertEqual({
+        'from_dictionary',
+        'get_all_options',
+        'slices',
+        'style',
+        'view_as',
+        'display_data'
+    },
+                     {
+                         attr
+                         for attr in dir(options)
+                         if not attr.startswith('_') and attr != 'next'
+                     })
+    self.assertEqual({
+        'from_dictionary',
+        'get_all_options',
+        'style',
+        'view_as',
+        'display_data'
+    },
+                     {
+                         attr
+                         for attr in dir(options.view_as(Eggs))
+                         if not attr.startswith('_') and attr != 'next'
+                     })
 
 
 class RunnerApiTest(unittest.TestCase):
diff --git a/sdks/python/apache_beam/runners/interactive/interactive_beam.py b/sdks/python/apache_beam/runners/interactive/interactive_beam.py
index 40f9774..421e8f9 100644
--- a/sdks/python/apache_beam/runners/interactive/interactive_beam.py
+++ b/sdks/python/apache_beam/runners/interactive/interactive_beam.py
@@ -525,7 +525,7 @@ def head(pcoll, n=5, include_window_info=False):
   results = []
   for e in elements:
     results.append(e)
-    if len(results) >= n and n > 0:
+    if len(results) >= n > 0:
       break
 
   return elements_to_df(results, include_window_info=include_window_info)
diff --git a/sdks/python/apache_beam/runners/portability/fn_api_runner/translations.py b/sdks/python/apache_beam/runners/portability/fn_api_runner/translations.py
index 235aec8..14a9a9a 100644
--- a/sdks/python/apache_beam/runners/portability/fn_api_runner/translations.py
+++ b/sdks/python/apache_beam/runners/portability/fn_api_runner/translations.py
@@ -897,7 +897,8 @@ def expand_sdf(stages, context):
               setattr(proto, name, value)
           if 'unique_name' not in kwargs and hasattr(proto, 'unique_name'):
             proto.unique_name = unique_name(
-                set([p.unique_name for p in protos.values()]),
+                {p.unique_name
+                 for p in protos.values()},
                 original.unique_name + suffix)
           return new_id
 
diff --git a/sdks/python/apache_beam/runners/worker/opcounters_test.py b/sdks/python/apache_beam/runners/worker/opcounters_test.py
index 0ac092f..d3ab22d 100644
--- a/sdks/python/apache_beam/runners/worker/opcounters_test.py
+++ b/sdks/python/apache_beam/runners/worker/opcounters_test.py
@@ -67,7 +67,7 @@ class TransformIoCounterTest(unittest.TestCase):
     sampler.stop()
     sampler.commit_counters()
 
-    actual_counter_names = set([c.name for c in counter_factory.get_counters()])
+    actual_counter_names = {c.name for c in counter_factory.get_counters()}
     expected_counter_names = set([
         # Counter names for STEP 1
         counters.CounterName(