From 3c4e118f0e4b0898749edf7c56218feb21fc5975 Mon Sep 17 00:00:00 2001 From: Karim Nakad Date: Mon, 15 Jul 2019 14:32:54 -0700 Subject: [PATCH 1/3] change: enable wrong-import-order Pylint check Per PEP8: Imports should be grouped in the following order: 1- Standard library imports. 2- Related third party imports. 3- Local application/library specific imports. --- .pylintrc | 1 - src/sagemaker/fw_utils.py | 2 +- src/sagemaker/git_utils.py | 2 +- src/sagemaker/local/image.py | 2 +- src/sagemaker/predictor.py | 2 +- src/sagemaker/session.py | 2 +- src/sagemaker/user_agent.py | 2 +- 7 files changed, 6 insertions(+), 7 deletions(-) diff --git a/.pylintrc b/.pylintrc index fecdb49e56..130124da70 100644 --- a/.pylintrc +++ b/.pylintrc @@ -86,7 +86,6 @@ disable= import-error, # Since we run Pylint before any of our builds in tox, this will always fail protected-access, # TODO: Fix access abstract-method, # TODO: Fix abstract methods - wrong-import-order, # TODO: Fix import order useless-object-inheritance, # TODO: Remove unnecessary imports cyclic-import, # TODO: Resolve cyclic imports no-self-use, # TODO: Convert methods to functions where appropriate diff --git a/src/sagemaker/fw_utils.py b/src/sagemaker/fw_utils.py index 3b90f9c006..8deb803fa3 100644 --- a/src/sagemaker/fw_utils.py +++ b/src/sagemaker/fw_utils.py @@ -16,11 +16,11 @@ import os import re -import sagemaker.utils import shutil import tempfile from six.moves.urllib.parse import urlparse +import sagemaker.utils from sagemaker.utils import get_ecr_image_uri_prefix, ECR_URI_PATTERN _TAR_SOURCE_FILENAME = "source.tar.gz" diff --git a/src/sagemaker/git_utils.py b/src/sagemaker/git_utils.py index 8028243b1f..8490ec5788 100644 --- a/src/sagemaker/git_utils.py +++ b/src/sagemaker/git_utils.py @@ -13,10 +13,10 @@ from __future__ import absolute_import import os -import six import subprocess import tempfile import warnings +import six from six.moves import urllib diff --git a/src/sagemaker/local/image.py b/src/sagemaker/local/image.py index 95a6277f87..bcc2bfab36 100644 --- a/src/sagemaker/local/image.py +++ b/src/sagemaker/local/image.py @@ -28,8 +28,8 @@ import tarfile import tempfile -from six.moves.urllib.parse import urlparse from threading import Thread +from six.moves.urllib.parse import urlparse import yaml diff --git a/src/sagemaker/predictor.py b/src/sagemaker/predictor.py index 68efe14d20..d2c4822289 100644 --- a/src/sagemaker/predictor.py +++ b/src/sagemaker/predictor.py @@ -15,9 +15,9 @@ import codecs import csv import json -import numpy as np import six from six import StringIO, BytesIO +import numpy as np from sagemaker.content_types import CONTENT_TYPE_JSON, CONTENT_TYPE_CSV, CONTENT_TYPE_NPY from sagemaker.session import Session diff --git a/src/sagemaker/session.py b/src/sagemaker/session.py index 019f9e4ef9..e0e30e66fc 100644 --- a/src/sagemaker/session.py +++ b/src/sagemaker/session.py @@ -20,9 +20,9 @@ import time import warnings +import six import boto3 import botocore.config -import six import yaml from botocore.exceptions import ClientError diff --git a/src/sagemaker/user_agent.py b/src/sagemaker/user_agent.py index a63d2bb6bb..2420266572 100644 --- a/src/sagemaker/user_agent.py +++ b/src/sagemaker/user_agent.py @@ -12,9 +12,9 @@ # language governing permissions and limitations under the License. from __future__ import absolute_import -import pkg_resources import platform import sys +import pkg_resources import boto3 import botocore From 6fedceb3c3cef05b72628b33b49bffaf73f24efa Mon Sep 17 00:00:00 2001 From: Karim Nakad Date: Mon, 15 Jul 2019 14:44:25 -0700 Subject: [PATCH 2/3] change: enable ungrouped-imports Pylint check --- .pylintrc | 1 - src/sagemaker/session.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.pylintrc b/.pylintrc index 130124da70..a8308f4400 100644 --- a/.pylintrc +++ b/.pylintrc @@ -91,7 +91,6 @@ disable= no-self-use, # TODO: Convert methods to functions where appropriate consider-using-in, # TODO: Consider merging comparisons with "in" too-many-public-methods, # TODO: Resolve - ungrouped-imports, # TODO: Group imports consider-using-ternary, # TODO: Consider ternary expressions chained-comparison, # TODO: Simplify chained comparison between operands too-many-branches, # TODO: Simplify or ignore as appropriate diff --git a/src/sagemaker/session.py b/src/sagemaker/session.py index e0e30e66fc..b39e0be055 100644 --- a/src/sagemaker/session.py +++ b/src/sagemaker/session.py @@ -23,8 +23,8 @@ import six import boto3 import botocore.config -import yaml from botocore.exceptions import ClientError +import yaml import sagemaker.logs from sagemaker import vpc_utils From 5d4d1274071729ca9f1987021ebe14ce10749726 Mon Sep 17 00:00:00 2001 From: Karim Nakad Date: Mon, 15 Jul 2019 15:04:43 -0700 Subject: [PATCH 3/3] change: enable consider-using-in Pylint check --- .pylintrc | 1 - src/sagemaker/session.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.pylintrc b/.pylintrc index a8308f4400..056ee6d841 100644 --- a/.pylintrc +++ b/.pylintrc @@ -89,7 +89,6 @@ disable= useless-object-inheritance, # TODO: Remove unnecessary imports cyclic-import, # TODO: Resolve cyclic imports no-self-use, # TODO: Convert methods to functions where appropriate - consider-using-in, # TODO: Consider merging comparisons with "in" too-many-public-methods, # TODO: Resolve consider-using-ternary, # TODO: Consider ternary expressions chained-comparison, # TODO: Simplify chained comparison between operands diff --git a/src/sagemaker/session.py b/src/sagemaker/session.py index b39e0be055..1e904d58d6 100644 --- a/src/sagemaker/session.py +++ b/src/sagemaker/session.py @@ -1022,7 +1022,7 @@ def _check_job_status(self, job, desc, status_key_name): # If the status is capital case, then convert it to Camel case status = _STATUS_CODE_TABLE.get(status, status) - if status != "Completed" and status != "Stopped": + if status not in ("Completed", "Stopped"): reason = desc.get("FailureReason", "(No reason provided)") job_type = status_key_name.replace("JobStatus", " job") raise ValueError("Error for {} {}: {} Reason: {}".format(job_type, job, status, reason)) @@ -1292,7 +1292,7 @@ def logs_for_job( # noqa: C901 - suppress complexity warning for this method client = self.boto_session.client("logs", config=config) log_group = "/aws/sagemaker/TrainingJobs" - job_already_completed = status == "Completed" or status == "Failed" or status == "Stopped" + job_already_completed = status in ("Completed", "Failed", "Stopped") state = LogState.TAILING if wait and not job_already_completed else LogState.COMPLETE dot = False @@ -1385,7 +1385,7 @@ def logs_for_job( # noqa: C901 - suppress complexity warning for this method status = description["TrainingJobStatus"] - if status == "Completed" or status == "Failed" or status == "Stopped": + if status in ("Completed", "Failed", "Stopped"): print() state = LogState.JOB_COMPLETE