Lint Python code for undefined names (#1721)

* Lint Python code for undefined names

* Lint Python code for undefined names

* Exclude tfdv.py to workaround an overzealous pytest

* Fixup for tfdv.py

* Fixup for tfdv.py

* Fixup for tfdv.py

.travis.yml

@@ -94,5 +94,9 @@ matrix:
     - language: python
       python: "3.7"
       env: TOXENV=py37
-      dist: xenial # required for Python >= 3.7
       script: *1
+    - name: "Lint Python code with flake8"
+      language: python
+      python: "3.7"
+      install: pip install flake8
+      script: flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics

backend/src/apiserver/visualization/roc_curve.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# flake8: noqa TODO
+
 import json
 from pathlib import Path
 from bokeh.layouts import row
@@ -34,7 +36,7 @@ from tensorflow.python.lib.io import file_io
 # trueclass
 # true_score_column
 
-if "is_generated" is not in variables or variables["is_generated"] is False:
+if not variables.get("is_generated"):
     # Create data from specified csv file(s).
     # The schema file provides column names for the csv file that will be used
     # to generate the roc curve.

backend/src/apiserver/visualization/snapshots/snap_test_exporter.py

@@ -76,7 +76,13 @@ snapshots['TestExporterMethods::test_create_cell_from_args_with_one_arg 1'] = ''
 
 '''
 
-snapshots['TestExporterMethods::test_create_cell_from_file 1'] = '''# Copyright 2019 Google LLC
+snapshots['TestExporterMethods::test_create_cell_from_file 1'] = '''"""
+test.py is used for test_server.py as a way to test the tornado web server
+without having a reliance on the validity of visualizations. It does not serve
+as a valid visualization and is only used for testing purposes.
+"""
+
+# Copyright 2019 Google LLC
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -90,17 +96,8 @@ snapshots['TestExporterMethods::test_create_cell_from_file 1'] = '''# Copyright
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import tensorflow_data_validation as tfdv
-
-# The following variables are provided through dependency injection. These
-# variables come from the specified input path and arguments provided by the
-# API post request.
-#
-# source
-
-train_stats = tfdv.generate_statistics_from_csv(data_location=source)
-
-tfdv.visualize_statistics(train_stats)
+x = 2
+print(x)
 '''
 
 snapshots['TestExporterMethods::test_generate_html_from_notebook 1'] = '''

backend/src/apiserver/visualization/test_exporter.py

@@ -58,7 +58,7 @@ class TestExporterMethods(snapshottest.TestCase):
 
     def test_create_cell_from_file(self):
         self.maxDiff = None
-        cell = self.exporter.create_cell_from_file("tfdv.py")
+        cell = self.exporter.create_cell_from_file("test.py")
         self.assertMatchSnapshot(cell.source)
 
     def test_generate_html_from_notebook(self):

backend/src/apiserver/visualization/tfdv.py

@@ -20,6 +20,6 @@ import tensorflow_data_validation as tfdv
 #
 # source
 
-train_stats = tfdv.generate_statistics_from_csv(data_location=source)
+train_stats = tfdv.generate_statistics_from_csv(data_location=source)  # noqa: F821
 
 tfdv.visualize_statistics(train_stats)

components/arena/python/arena/_arena_distributed_tf_op.py

@@ -13,20 +13,21 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# flake8: noqa TODO
 
 import kfp.dsl as dsl
 import datetime
 import logging
 
 
-def estimator_op(name, image, command, 
+def estimator_op(name, image, command,
                       chief_cpu_limit, chief_memory_limit, chief_port,
                       workers, worker_image, worker_cpu_limit, worker_memory_limit,
-                      parameter_servers, ps_image, ps_cpu_limit, ps_memory_limit, ps_port, 
-                      gpus, rdma, 
-                      tensorboard, 
+                      parameter_servers, ps_image, ps_cpu_limit, ps_memory_limit, ps_port,
+                      gpus, rdma,
+                      tensorboard,
                       worker_port, annotations=[],
-                      evaluator=False, evaluator_cpu_limit='0', evaluator_memory_limit='0', 
+                      evaluator=False, evaluator_cpu_limit='0', evaluator_memory_limit='0',
                       env=[], data=[], sync_source=None,
                       metrics=['Train-accuracy:PERCENTAGE'],
                       arena_image='cheyang/arena_launcher:v0.7',
@@ -44,11 +45,11 @@ def estimator_op(name, image, command,
                       workers=workers, worker_image=worker_image, worker_cpu_limit=worker_cpu_limit, worker_memory_limit=worker_memory,
                       parameter_servers=parameter_servers, ps_image=ps_image, ps_cpu_limit=ps_cpu_limit, ps_memory_limit=ps_memory_limit,
                       gpus=gpus, rdma=rdma,
-                      chief=True, 
+                      chief=True,
                       chief_cpu_limit=chief_cpu_limit,
                       worker_port=worker_port,
                       ps_port=ps_port,
-                      tensorboard=tensorboard, 
+                      tensorboard=tensorboard,
                       metrics=metrics,
                       arena_image=arena_image,
                       timeout_hours=timeout_hours)
@@ -58,9 +59,9 @@ def estimator_op(name, image, command,
 def parameter_servers_op(name, image, command, env, data, sync_source, annotations,
                       workers, worker_image, worker_cpu_limit, worker_memory,
                       parameter_servers, ps_image, ps_cpu_limit, ps_memory_limit,
-                      gpus, rdma, 
-                      tensorboard, 
-                      worker_port, ps_port, 
+                      gpus, rdma,
+                      tensorboard,
+                      worker_port, ps_port,
                       metrics=['Train-accuracy:PERCENTAGE'],
                       arena_image='cheyang/arena_launcher:v0.7',
                       timeout_hours=240):
@@ -76,10 +77,10 @@ def parameter_servers_op(name, image, command, env, data, sync_source, annotatio
     return distributed_tf_op(name=name, image=image, command=command, envs=envs, data=data, sync_source=sync_source,
                       workers=workers, worker_image=worker_image, worker_cpu_limit=worker_cpu_limit, worker_memory_limit=worker_memory,
                       parameter_servers=parameter_servers, ps_image=ps_image, ps_cpu_limit=ps_cpu_limit, ps_memory_limit=ps_memory_limit,
-                      gpus=gpus, rdma=rdma, 
+                      gpus=gpus, rdma=rdma,
                       worker_port=worker_port,
                       ps_port=ps_port,
-                      tensorboard=tensorboard, 
+                      tensorboard=tensorboard,
                       metrics=metrics,
                       arena_image=arena_image,
                       timeout_hours=timeout_hours)
@@ -87,15 +88,15 @@ def parameter_servers_op(name, image, command, env, data, sync_source, annotatio
 
 
 def distributed_tf_op(name, image, command, env=[], data=[], sync_source=None,
-                      chief=False, chief_cpu_limit='0', chief_memory_limit='0', 
+                      chief=False, chief_cpu_limit='0', chief_memory_limit='0',
                       workers=0, worker_image=None, worker_cpu_limit='0', worker_memory_limit='0',
                       parameter_servers=0, ps_image=None, ps_cpu_limit='0', ps_memory_limit='0',
-                      evaluator=False, evaluator_cpu_limit='0', evaluator_memory_limit='0', 
-                      gpus=0, rdma=False, 
+                      evaluator=False, evaluator_cpu_limit='0', evaluator_memory_limit='0',
+                      gpus=0, rdma=False,
                       chief_port=22222,
                       worker_port=22222,
                       ps_port=22224,
-                      tensorboard=False, 
+                      tensorboard=False,
                       metrics=['Train-accuracy:PERCENTAGE'],
                       arena_image='cheyang/arena_launcher:v0.7',
                       timeout_hours=240):

components/aws/emr/create_cluster/src/create_cluster.py

@@ -17,6 +17,12 @@ from pathlib2 import Path
 
 from common import _utils
 
+try:
+  unicode
+except NameError:
+  unicode = str
+
+
 def main(argv=None):
   parser = argparse.ArgumentParser(description='Create EMR Cluster')
   parser.add_argument('--region', type=str, help='EMR Cluster region.')

components/aws/emr/submit_pyspark_job/src/submit_pyspark_job.py

@@ -29,6 +29,11 @@ from pathlib2 import Path
 
 from common import _utils
 
+try:
+  unicode
+except NameError:
+  unicode = str
+
 
 def main(argv=None):
   parser = argparse.ArgumentParser(description='Submit PySpark Job')

components/aws/emr/submit_spark_job/src/submit_spark_job.py

@@ -30,6 +30,11 @@ from pathlib2 import Path
 
 from common import _utils
 
+try:
+  unicode
+except NameError:
+  unicode = str
+
 
 def main(argv=None):
   parser = argparse.ArgumentParser(description='Submit Spark Job')

components/aws/sagemaker/batch_transform/src/batch_transform.py

@@ -16,6 +16,12 @@ from pathlib2 import Path
 
 from common import _utils
 
+try:
+  unicode
+except NameError:
+  unicode = str
+
+
 def main(argv=None):
   parser = argparse.ArgumentParser(description='SageMaker Batch Transformation Job')
   parser.add_argument('--region', type=str.strip, required=True, help='The region where the cluster launches.')

proxy/get_proxy_url.py

@@ -21,6 +21,11 @@ import logging
 import re
 import requests
 
+try:
+  unicode
+except NameError:
+  unicode = str
+
 def urls_for_zone(zone, location_to_urls_map):
   """Returns list of potential proxy URLs for a given zone.
 

samples/contrib/aws-samples/ground_truth_pipeline_demo/mini-image-classification-pipeline.py

@@ -136,4 +136,4 @@ def ground_truth_test(region='us-west-2',
     ).apply(use_aws_secret('aws-secret', 'AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY'))
 
 if __name__ == '__main__':
-    kfp.compiler.Compiler().compile(hpo_test, __file__ + '.zip')
+    kfp.compiler.Compiler().compile(hpo_test, __file__ + '.zip')  # noqa: F821 TODO

samples/contrib/resnet-cmle/resnet-train-pipeline.py

@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+# flake8: noqa TODO
 
 import kfp.dsl as dsl
 import kfp.gcp as gcp

sdk/python/kfp/components/_dynamic.py

@@ -28,9 +28,9 @@ def create_function_from_parameter_names(func: Callable[[Mapping[str, Any]], Any
 
 def create_function_from_parameters(func: Callable[[Mapping[str, Any]], Any], parameters: Sequence[Parameter], documentation=None, func_name=None, func_filename=None):
     new_signature = Signature(parameters) # Checks the parameter consistency
-    
+
     def pass_locals():
-        return dict_func(locals())
+        return dict_func(locals())  # noqa: F821 TODO
 
     code = pass_locals.__code__
     mod_co_argcount = len(parameters)
@@ -59,10 +59,10 @@ def create_function_from_parameters(func: Callable[[Mapping[str, Any]], Any], pa
         mod_co_firstlineno,
         code.co_lnotab
     )
-    
+
     default_arg_values = tuple( p.default for p in parameters if p.default != Parameter.empty ) #!argdefs "starts from the right"/"is right-aligned"
     modified_func = types.FunctionType(modified_code, {'dict_func': func, 'locals': locals}, name=func_name, argdefs=default_arg_values)
     modified_func.__doc__ = documentation
     modified_func.__signature__ = new_signature
-    
+
     return modified_func

sdk/python/kfp/dsl/_pipeline.py

@@ -47,7 +47,7 @@ def pipeline(name : str = None, description : str = None):
     if _pipeline_decorator_handler:
       return _pipeline_decorator_handler(func) or func
     else:
-      return func 
+      return func
 
   return _pipeline
 
@@ -88,7 +88,7 @@ class PipelineConf():
       seconds: number of seconds for the workflow to be garbage collected after it is finished.
     """
     self.ttl_seconds_after_finished = seconds
-    return self   
+    return self
 
   def set_artifact_location(self, artifact_location):
     """Configures the pipeline level artifact location.
@@ -243,7 +243,7 @@ class Pipeline():
     Args:
       metadata (ComponentMeta): component metadata
     '''
-    if not isinstance(metadata, PipelineMeta):
+    if not isinstance(metadata, PipelineMeta):  # noqa: F821 TODO
       raise ValueError('_set_medata is expecting PipelineMeta.')
     self._metadata = metadata
 

sdk/python/tests/dsl/component_tests.py

@@ -63,7 +63,7 @@ class TestPythonComponent(unittest.TestCase):
 
     @component
     def b_op(field_x: {'customized_type': {'property_a': 'value_a', 'property_b': 'value_b'}},
-        field_y: 'GcsUri',
+        field_y: 'GcsUri',  # noqa: F821 TODO
         field_z: GCSPath()) -> {'output_model_uri': 'GcsUri'}:
       return ContainerOp(
           name = 'operator b',