From 6e5a7948cdb1ad6ba04384d9de4322216e2586e2 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Fri, 12 Jun 2020 10:52:32 -0700 Subject: [PATCH 1/8] fix: set logs to False if wait is False --- src/sagemaker/automl/automl.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index 794e03aee1..8ce6f15955 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -78,16 +78,13 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): is stored. Or an AutoMLInput object. If a local path is provided, the dataset will be uploaded to an S3 location. wait (bool): Whether the call should wait until the job completes (default: True). - logs (bool): Whether to show the logs produced by the job. - Only meaningful when wait is True (default: True). + logs (bool): Whether to show the logs produced by the job. Only meaningful when wait + is True (default: True). if `wait` is False, `log` will be set to False as well. job_name (str): Training job name. If not specified, the estimator generates a default job name, based on the training image name and current timestamp. """ - if logs and not wait: - raise ValueError( - """Logs can only be shown if wait is set to True. - Please either set wait to True or set logs to False.""" - ) + if not wait: + logs = False # upload data for users if provided local path # validations are done in _Job._format_inputs_to_input_config From 7751e623cdec57fc27bf73018c7e264dc04221d5 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Mon, 15 Jun 2020 12:02:07 -0700 Subject: [PATCH 2/8] update unit test to test not raising error when wait is False logs is True --- src/sagemaker/automl/automl.py | 2 +- tests/unit/sagemaker/automl/test_auto_ml.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index 8ce6f15955..6440bb676b 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -79,7 +79,7 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): be uploaded to an S3 location. wait (bool): Whether the call should wait until the job completes (default: True). logs (bool): Whether to show the logs produced by the job. Only meaningful when wait - is True (default: True). if `wait` is False, `log` will be set to False as well. + is True (default: True). if `wait` is False, `logs` will be set to False as well. job_name (str): Training job name. If not specified, the estimator generates a default job name, based on the training image name and current timestamp. """ diff --git a/tests/unit/sagemaker/automl/test_auto_ml.py b/tests/unit/sagemaker/automl/test_auto_ml.py index 8ef0cd31da..7fd518857e 100644 --- a/tests/unit/sagemaker/automl/test_auto_ml.py +++ b/tests/unit/sagemaker/automl/test_auto_ml.py @@ -248,7 +248,7 @@ def test_auto_ml_additional_optional_params(sagemaker_session): tags=TAGS, ) inputs = DEFAULT_S3_INPUT_DATA - auto_ml.fit(inputs, job_name=JOB_NAME) + auto_ml.fit(inputs, job_name=JOB_NAME, wait=False, logs=True) sagemaker_session.auto_ml.assert_called_once() _, args = sagemaker_session.auto_ml.call_args From c066263c2dd0d080f001afe3d29cfdc9e84ab6b1 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Mon, 15 Jun 2020 12:26:54 -0700 Subject: [PATCH 3/8] add warning --- src/sagemaker/automl/automl.py | 8 +++++++- tests/unit/sagemaker/automl/test_auto_ml.py | 3 ++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index 6440bb676b..d17c13e5ee 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -13,6 +13,7 @@ """A class for SageMaker AutoML Jobs.""" from __future__ import absolute_import +import logging from six import string_types from sagemaker import Model, PipelineModel @@ -21,6 +22,8 @@ from sagemaker.session import Session from sagemaker.utils import name_from_base +logger = logging.getLogger("sagemaker") + class AutoML(object): """A class for creating and interacting with SageMaker AutoML jobs @@ -83,8 +86,11 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): job_name (str): Training job name. If not specified, the estimator generates a default job name, based on the training image name and current timestamp. """ - if not wait: + if not wait and logs: logs = False + logger.warning( + "logs will be set to False. logs is only meaningful when wait is True." + ) # upload data for users if provided local path # validations are done in _Job._format_inputs_to_input_config diff --git a/tests/unit/sagemaker/automl/test_auto_ml.py b/tests/unit/sagemaker/automl/test_auto_ml.py index 7fd518857e..d3a414b3f4 100644 --- a/tests/unit/sagemaker/automl/test_auto_ml.py +++ b/tests/unit/sagemaker/automl/test_auto_ml.py @@ -228,7 +228,7 @@ def test_auto_ml_only_one_of_problem_type_and_job_objective_provided(sagemaker_s ) -def test_auto_ml_additional_optional_params(sagemaker_session): +def test_auto_ml_additional_optional_params(sagemaker_session, caplog): auto_ml = AutoML( role=ROLE, target_attribute_name=TARGET_ATTRIBUTE_NAME, @@ -282,6 +282,7 @@ def test_auto_ml_additional_optional_params(sagemaker_session): "generate_candidate_definitions_only": GENERATE_CANDIDATE_DEFINITIONS_ONLY, "tags": TAGS, } + assert "logs will be set to False. logs is only meaningful when wait is True." in caplog.text @patch("time.strftime", return_value=TIMESTAMP) From 60464defe9fbce4b38e0ecac431489b8dd924967 Mon Sep 17 00:00:00 2001 From: Chuyang Deng Date: Mon, 15 Jun 2020 12:33:58 -0700 Subject: [PATCH 4/8] black check --- src/sagemaker/automl/automl.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index d17c13e5ee..d0e53750ec 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -88,9 +88,7 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): """ if not wait and logs: logs = False - logger.warning( - "logs will be set to False. logs is only meaningful when wait is True." - ) + logger.warning("logs will be set to False. logs is only meaningful when wait is True.") # upload data for users if provided local path # validations are done in _Job._format_inputs_to_input_config From b8e8d610aca0bb488d836fb681e231f8e4434a5a Mon Sep 17 00:00:00 2001 From: Chuyang Date: Mon, 15 Jun 2020 16:00:43 -0700 Subject: [PATCH 5/8] test wait not called --- tests/unit/sagemaker/automl/test_auto_ml.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/tests/unit/sagemaker/automl/test_auto_ml.py b/tests/unit/sagemaker/automl/test_auto_ml.py index 553fd3533a..9807ca56fe 100644 --- a/tests/unit/sagemaker/automl/test_auto_ml.py +++ b/tests/unit/sagemaker/automl/test_auto_ml.py @@ -294,7 +294,18 @@ def test_auto_ml_only_one_of_problem_type_and_job_objective_provided(sagemaker_s ) -def test_auto_ml_additional_optional_params(sagemaker_session, caplog): +@patch("sagemaker.automl.automl.AutoMLJob.start_new") +def test_auto_ml_fit_set_logs_to_false(start_new, sagemaker_session, caplog): + auto_ml = AutoML( + role=ROLE, target_attribute_name=TARGET_ATTRIBUTE_NAME, sagemaker_session=sagemaker_session + ) + inputs = DEFAULT_S3_INPUT_DATA + auto_ml.fit(inputs, job_name=JOB_NAME, wait=False, logs=True) + start_new.wait.assert_not_called() + assert "Setting logs to False. logs is only meaningful when wait is True." in caplog.text + + +def test_auto_ml_additional_optional_params(sagemaker_session): auto_ml = AutoML( role=ROLE, target_attribute_name=TARGET_ATTRIBUTE_NAME, @@ -314,7 +325,7 @@ def test_auto_ml_additional_optional_params(sagemaker_session, caplog): tags=TAGS, ) inputs = DEFAULT_S3_INPUT_DATA - auto_ml.fit(inputs, job_name=JOB_NAME, wait=False, logs=True) + auto_ml.fit(inputs, job_name=JOB_NAME) sagemaker_session.auto_ml.assert_called_once() _, args = sagemaker_session.auto_ml.call_args @@ -348,7 +359,6 @@ def test_auto_ml_additional_optional_params(sagemaker_session, caplog): "generate_candidate_definitions_only": GENERATE_CANDIDATE_DEFINITIONS_ONLY, "tags": TAGS, } - assert "logs will be set to False. logs is only meaningful when wait is True." in caplog.text @patch("time.strftime", return_value=TIMESTAMP) From 1de42cb74bfdf36312674a635e6d28149b65125d Mon Sep 17 00:00:00 2001 From: Chuyang Date: Mon, 15 Jun 2020 16:19:39 -0700 Subject: [PATCH 6/8] update warning message --- src/sagemaker/automl/automl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index 07453dfd05..79bd1f3eab 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -82,13 +82,13 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): be uploaded to an S3 location. wait (bool): Whether the call should wait until the job completes (default: True). logs (bool): Whether to show the logs produced by the job. Only meaningful when wait - is True (default: True). if `wait` is False, `logs` will be set to False as well. + is True (default: True). if ``wait`` is False, ``logs`` will be set to False as well. job_name (str): Training job name. If not specified, the estimator generates a default job name, based on the training image name and current timestamp. """ if not wait and logs: logs = False - logger.warning("logs will be set to False. logs is only meaningful when wait is True.") + logger.warning("Setting logs to False. logs is only meaningful when wait is True.") # upload data for users if provided local path # validations are done in _Job._format_inputs_to_input_config From 151e24cf3185819b7cd33d45663eb9b427816b15 Mon Sep 17 00:00:00 2001 From: Chuyang Date: Mon, 15 Jun 2020 22:26:48 -0700 Subject: [PATCH 7/8] fix pylint error --- src/sagemaker/automl/automl.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index 79bd1f3eab..6ff646eb37 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -82,7 +82,8 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): be uploaded to an S3 location. wait (bool): Whether the call should wait until the job completes (default: True). logs (bool): Whether to show the logs produced by the job. Only meaningful when wait - is True (default: True). if ``wait`` is False, ``logs`` will be set to False as well. + is True (default: True). if ``wait`` is False, ``logs`` will be set to False as + well. job_name (str): Training job name. If not specified, the estimator generates a default job name, based on the training image name and current timestamp. """ From 140135142f75e86833e72ae4ed980223cce7d395 Mon Sep 17 00:00:00 2001 From: Chuyang Date: Tue, 16 Jun 2020 11:29:11 -0700 Subject: [PATCH 8/8] remove trailing whitespace --- src/sagemaker/automl/automl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sagemaker/automl/automl.py b/src/sagemaker/automl/automl.py index 6ff646eb37..ed2a1fcc7c 100644 --- a/src/sagemaker/automl/automl.py +++ b/src/sagemaker/automl/automl.py @@ -82,7 +82,7 @@ def fit(self, inputs=None, wait=True, logs=True, job_name=None): be uploaded to an S3 location. wait (bool): Whether the call should wait until the job completes (default: True). logs (bool): Whether to show the logs produced by the job. Only meaningful when wait - is True (default: True). if ``wait`` is False, ``logs`` will be set to False as + is True (default: True). if ``wait`` is False, ``logs`` will be set to False as well. job_name (str): Training job name. If not specified, the estimator generates a default job name, based on the training image name and current timestamp.