Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint Python code for undefined names #1721

Merged
merged 6 commits into from
Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 5 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,9 @@ matrix:
- language: python
python: "3.7"
env: TOXENV=py37
dist: xenial # required for Python >= 3.7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this no longer necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

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
4 changes: 3 additions & 1 deletion backend/src/apiserver/visualization/roc_curve.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -34,7 +36,7 @@
# trueclass
# true_score_column

if "is_generated" is not in variables or variables["is_generated"] is False:
if not variables.get("is_generated"):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should revert this change and move it to a separate PR. What are your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That change has been in #1878 for 4 days (a long time for a syntax error) and was not acted on so I bundled it it here

# 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,13 @@

'''

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.
Expand All @@ -90,17 +96,8 @@
# 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'] = '''
Expand Down
2 changes: 1 addition & 1 deletion backend/src/apiserver/visualization/test_exporter.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def test_create_cell_from_args_with_multiple_args(self):

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):
Expand Down
2 changes: 1 addition & 1 deletion backend/src/apiserver/visualization/tfdv.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@
#
# 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)
33 changes: 17 additions & 16 deletions components/arena/python/arena/_arena_distributed_tf_op.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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)
Expand All @@ -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):
Expand All @@ -76,26 +77,26 @@ 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)



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):
Expand Down
6 changes: 6 additions & 0 deletions components/aws/emr/create_cluster/src/create_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@

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.')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@

from common import _utils

try:
unicode
except NameError:
unicode = str


def main(argv=None):
parser = argparse.ArgumentParser(description='Submit PySpark Job')
Expand Down
5 changes: 5 additions & 0 deletions components/aws/emr/submit_spark_job/src/submit_spark_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@

from common import _utils

try:
unicode
except NameError:
unicode = str


def main(argv=None):
parser = argparse.ArgumentParser(description='Submit Spark Job')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@

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.')
Expand Down
5 changes: 5 additions & 0 deletions proxy/get_proxy_url.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions samples/contrib/resnet-cmle/resnet-train-pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions sdk/python/kfp/components/_dynamic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -59,10 +59,10 @@ def pass_locals():
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
6 changes: 3 additions & 3 deletions sdk/python/kfp/dsl/_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def _pipeline(func):
if _pipeline_decorator_handler:
return _pipeline_decorator_handler(func) or func
else:
return func
return func

return _pipeline

Expand Down Expand Up @@ -88,7 +88,7 @@ def set_ttl_seconds_after_finished(self, seconds: int):
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.
Expand Down Expand Up @@ -243,7 +243,7 @@ def _set_metadata(self, metadata):
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

Expand Down
2 changes: 1 addition & 1 deletion sdk/python/tests/dsl/component_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def a_op(field_l: Integer()) -> {'field_m': GCSPath(), 'field_n': {'customized_t

@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',
Expand Down