Skip to content

fix: add v2 warning messages #1480

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

Merged
merged 25 commits into from
May 20, 2020
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6e19e89
fix: add v2 warning messages
May 8, 2020
b4da387
Merge branch 'master' into warning
chuyang-deng May 8, 2020
0dd827d
Merge branch 'master' into warning
chuyang-deng May 10, 2020
d4c151d
Merge branch 'master' into warning
chuyang-deng May 12, 2020
ed1ee95
fix flake8 error
May 12, 2020
df90185
Merge branch 'master' into warning
chuyang-deng May 12, 2020
f7f0ac6
change: create ASTTransformer class to handle migrating Python SDK co…
laurenyu May 14, 2020
81ad62b
change: add class to read Python scripts and update code for v2 (#1497)
laurenyu May 15, 2020
d197b74
infra: add tools/ dir to pylint check (#1499)
laurenyu May 15, 2020
88518e0
change: add CLI wrapper for v2 migration script (#1500)
laurenyu May 18, 2020
57b2a22
change: add .ipynb file support for v2 migration script (#1508)
laurenyu May 19, 2020
4999c58
update with aws master
chuyang-deng May 19, 2020
e32ca32
update with aws zwei
chuyang-deng May 19, 2020
0701477
Revert "update with aws zwei"
chuyang-deng May 19, 2020
d99a83a
Merge pull request #6 from ChuyangDeng/revert-5-zwei
chuyang-deng May 19, 2020
dbeaa95
fix flake8 error
May 12, 2020
187d4f0
address comments
May 19, 2020
aa5709a
Merge branch 'warning' of github.com:ChuyangDeng/sagemaker-python-sdk…
May 19, 2020
6cf5322
fix pylint error
May 20, 2020
3b0ddb5
Merge branch 'master' into warning
chuyang-deng May 20, 2020
bba3554
remove eval_metrics warning
May 20, 2020
3420def
Merge branch 'warning' of github.com:ChuyangDeng/sagemaker-python-sdk…
May 20, 2020
4aa98e3
wrap session warning
May 20, 2020
17e7540
fix black error
May 20, 2020
2e0866f
Merge branch 'master' into warning
laurenyu May 20, 2020
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
9 changes: 9 additions & 0 deletions src/sagemaker/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"""Placeholder docstring"""
from __future__ import absolute_import

import logging
import sys
import importlib_metadata

from sagemaker import estimator, parameter, tuner # noqa: F401
Expand Down Expand Up @@ -61,3 +63,10 @@
from sagemaker.automl.candidate_estimator import CandidateEstimator, CandidateStep # noqa: F401

__version__ = importlib_metadata.version("sagemaker")

if sys.version[0] == "2":
logging.getLogger("sagemaker").warning(
"SageMaker Python SDK v2 will no longer support Python 2. "
"Please see https://github.com/aws/sagemaker-python-sdk/issues/1459 "
"for more information"
)
5 changes: 5 additions & 0 deletions src/sagemaker/amazon/amazon_estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,11 @@ def get_image_uri(region_name, repo_name, repo_version=1):
repo_name:
repo_version:
"""
logger.warning(
"'get_image_uri' method will be deprecated in favor of 'ImageURIProvider' class "
"in SageMaker Python SDK v2."
)

if repo_name == "xgboost":
if not _is_latest_xgboost_version(repo_version):
logging.warning(
Expand Down
11 changes: 11 additions & 0 deletions src/sagemaker/amazon/kmeans.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"""Placeholder docstring"""
from __future__ import absolute_import

import logging

from sagemaker.amazon.amazon_estimator import AmazonAlgorithmEstimatorBase, registry
from sagemaker.amazon.common import numpy_to_record_serializer, record_deserializer
from sagemaker.amazon.hyperparameter import Hyperparameter as hp # noqa
Expand All @@ -23,6 +25,9 @@
from sagemaker.vpc_utils import VPC_CONFIG_DEFAULT


logger = logging.getLogger("sagemaker")


class KMeans(AmazonAlgorithmEstimatorBase):
"""Placeholder docstring"""

Expand Down Expand Up @@ -154,6 +159,12 @@ def __init__(
self.center_factor = center_factor
self.eval_metrics = eval_metrics

if eval_metrics is not None:
logger.warning(
"Parameter 'eval_metrics' hyperparameter will be deprecated for 1P estimators "
"in SageMaker Python SDK v2."
)

def create_model(self, vpc_config_override=VPC_CONFIG_DEFAULT, **kwargs):
"""Return a :class:`~sagemaker.amazon.kmeans.KMeansModel` referencing
the latest s3 model data produced by this Estimator.
Expand Down
11 changes: 11 additions & 0 deletions src/sagemaker/amazon/randomcutforest.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"""Placeholder docstring"""
from __future__ import absolute_import

import logging

from sagemaker.amazon.amazon_estimator import AmazonAlgorithmEstimatorBase, registry
from sagemaker.amazon.common import numpy_to_record_serializer, record_deserializer
from sagemaker.amazon.hyperparameter import Hyperparameter as hp # noqa
Expand All @@ -23,6 +25,9 @@
from sagemaker.vpc_utils import VPC_CONFIG_DEFAULT


logger = logging.getLogger("sagemaker")


class RandomCutForest(AmazonAlgorithmEstimatorBase):
"""Placeholder docstring"""

Expand Down Expand Up @@ -119,6 +124,12 @@ def __init__(
self.num_trees = num_trees
self.eval_metrics = eval_metrics

if eval_metrics is not None:
logger.warning(
"Parameter 'eval_metrics' hyperparameter will be deprecated for 1P estimators "
"in SageMaker Python SDK v2."
)

def create_model(self, vpc_config_override=VPC_CONFIG_DEFAULT, **kwargs):
"""Return a :class:`~sagemaker.amazon.RandomCutForestModel` referencing
the latest s3 model data produced by this Estimator.
Expand Down
4 changes: 4 additions & 0 deletions src/sagemaker/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
UploadedCode,
validate_source_dir,
_region_supports_debugger,
parameter_v2_rename_warning,
)
from sagemaker.job import _Job
from sagemaker.local import LocalSession
Expand Down Expand Up @@ -1273,6 +1274,7 @@ def __init__(
https://docs.aws.amazon.com/sagemaker/latest/dg/API_AlgorithmSpecification.html#SageMaker-Type-AlgorithmSpecification-EnableSageMakerMetricsTimeSeries
(default: ``None``).
"""
logging.warning(parameter_v2_rename_warning("image_name", "image_uri"))
self.image_name = image_name
self.hyperparam_dict = hyperparameters.copy() if hyperparameters else {}
super(Estimator, self).__init__(
Expand Down Expand Up @@ -1637,6 +1639,8 @@ def __init__(
self.container_log_level = container_log_level
self.code_location = code_location
self.image_name = image_name
if image_name is not None:
logging.warning(parameter_v2_rename_warning("image_name", "image_uri"))

self.uploaded_code = None

Expand Down
25 changes: 24 additions & 1 deletion src/sagemaker/fw_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@
instantiated with positional or keyword arguments.
"""

EMPTY_FRAMEWORK_VERSION_WARNING = "No framework_version specified, defaulting to version {}."
EMPTY_FRAMEWORK_VERSION_WARNING = (
"No framework_version specified, defaulting to version {}. "
"framework_version will be required in SageMaker Python SDK v2."
)
LATER_FRAMEWORK_VERSION_WARNING = (
"This is not the latest supported version. "
"If you would like to use version {latest}, "
Expand All @@ -52,6 +55,10 @@
"fully leverage all GPU cores; the parameter server will be configured to run "
"only one worker per host regardless of the number of GPUs."
)
PARAMETER_V2_RENAME_WARNING = (
"Parameter {v1_parameter_name} will be renamed to {v2_parameter_name} "
"in SageMaker Python SDK v2."
)


EMPTY_FRAMEWORK_VERSION_ERROR = (
Expand Down Expand Up @@ -253,6 +260,11 @@ def create_image_uri(
Returns:
str: The appropriate image URI based on the given parameters.
"""
logger.warning(
"'create_image_uri' will be deprecated in favor of 'ImageURIProvider' class "
"in SageMaker Python SDK v2."
)

optimized_families = optimized_families or []

if py_version and py_version not in VALID_PY_VERSIONS:
Expand Down Expand Up @@ -647,6 +659,17 @@ def python_deprecation_warning(framework, latest_supported_version):
)


def parameter_v2_rename_warning(v1_parameter_name, v2_parameter_name):
"""
Args:
v1_parameter_name: parameter name used in SageMaker Python SDK v1
v2_parameter_name: parameter name used in SageMaker Python SDK v2
"""
return PARAMETER_V2_RENAME_WARNING.format(
v1_parameter_name=v1_parameter_name, v2_parameter_name=v2_parameter_name
)


def _region_supports_debugger(region_name):
"""Returns boolean indicating whether the region supports Amazon SageMaker Debugger.

Expand Down
7 changes: 7 additions & 0 deletions src/sagemaker/inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,13 @@
"""Amazon SageMaker channel configurations for S3 data sources and file system data sources"""
from __future__ import absolute_import, print_function

import logging

FILE_SYSTEM_TYPES = ["FSxLustre", "EFS"]
FILE_SYSTEM_ACCESS_MODES = ["ro", "rw"]

logger = logging.getLogger("sagemaker")


class s3_input(object):
"""Amazon SageMaker channel configurations for S3 data sources.
Expand Down Expand Up @@ -76,6 +80,9 @@ def __init__(
this channel. See the SageMaker API documentation for more info:
https://docs.aws.amazon.com/sagemaker/latest/dg/API_ShuffleConfig.html
"""
logger.warning(
"'s3_input' class will be renamed to 'TrainingInput' in SageMaker Python SDK v2."
)

self.config = {
"DataSource": {"S3DataSource": {"S3DataType": s3_data_type, "S3Uri": s3_data}}
Expand Down
2 changes: 2 additions & 0 deletions src/sagemaker/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ def __init__(
model_kms_key (str): KMS key ARN used to encrypt the repacked
model archive file if the model is repacked
"""
LOGGER.warning(fw_utils.parameter_v2_rename_warning("image", "image_uri"))

self.model_data = model_data
self.image = image
self.role = role
Expand Down
2 changes: 2 additions & 0 deletions src/sagemaker/mxnet/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
framework_version_from_tag,
empty_framework_version_warning,
python_deprecation_warning,
parameter_v2_rename_warning,
is_version_equal_or_higher,
warn_if_parameter_server_with_multi_gpu,
)
Expand Down Expand Up @@ -129,6 +130,7 @@ def __init__(
)

if distributions is not None:
logger.warning(parameter_v2_rename_warning("distributions", "distribution"))
train_instance_type = kwargs.get("train_instance_type")
warn_if_parameter_server_with_multi_gpu(
training_instance_type=train_instance_type, distributions=distributions
Expand Down
21 changes: 21 additions & 0 deletions src/sagemaker/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,27 @@
"""This module contains Enums and helper methods related to S3."""
from __future__ import print_function, absolute_import

import logging
import os

from six.moves.urllib.parse import urlparse
from sagemaker.session import Session

logger = logging.getLogger("sagemaker")

SESSION_V2_RENAME_MESSAGE = (
"Parameter 'session' will be renamed to 'sagemaker_session' in SageMaker Python SDK v2."
)


def _session_v2_rename_warning(session):
"""
Args:
session (sagemaker.session.Session):
"""
if session is not None:
logger.warning(SESSION_V2_RENAME_MESSAGE)


def parse_s3_url(url):
"""Returns an (s3 bucket, key name/prefix) tuple from a url with an s3
Expand Down Expand Up @@ -54,6 +70,7 @@ def upload(local_path, desired_s3_uri, kms_key=None, session=None):
The S3 uri of the uploaded file(s).

"""
_session_v2_rename_warning(session)
Copy link
Contributor

Choose a reason for hiding this comment

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

let's wrap this in an if session is not None

(same below)

sagemaker_session = session or Session()
bucket, key_prefix = parse_s3_url(url=desired_s3_uri)
if kms_key is not None:
Expand All @@ -80,6 +97,7 @@ def upload_string_as_file_body(body, desired_s3_uri=None, kms_key=None, session=
str: The S3 uri of the uploaded file(s).

"""
_session_v2_rename_warning(session)
sagemaker_session = session or Session()
bucket, key = parse_s3_url(desired_s3_uri)

Expand Down Expand Up @@ -107,6 +125,7 @@ def download(s3_uri, local_path, kms_key=None, session=None):
using the default AWS configuration chain.

"""
_session_v2_rename_warning(session)
sagemaker_session = session or Session()
bucket, key_prefix = parse_s3_url(url=s3_uri)
if kms_key is not None:
Expand All @@ -131,6 +150,7 @@ def read_file(s3_uri, session=None):
str: The body of the file.

"""
_session_v2_rename_warning(session)
sagemaker_session = session or Session()
bucket, key_prefix = parse_s3_url(url=s3_uri)

Expand All @@ -149,6 +169,7 @@ def list(s3_uri, session=None):
[str]: The list of S3 URIs in the given S3 base uri.

"""
_session_v2_rename_warning(session)
sagemaker_session = session or Session()
bucket, key_prefix = parse_s3_url(url=s3_uri)

Expand Down
13 changes: 13 additions & 0 deletions src/sagemaker/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ def upload_data(self, path, bucket=None, key_prefix="data", extra_args=None):
``s3://{bucket name}/{key_prefix}``.
"""
# Generate a tuple for each file that we want to upload of the form (local_path, s3_key).
LOGGER.warning(
"'upload_data' method will be deprecated in favor of 'S3Uploader' class "
"(https://sagemaker.readthedocs.io/en/stable/s3.html#sagemaker.s3.S3Uploader) "
"in SageMaker Python SDK v2."
)

files = []
key_suffix = None
if os.path.isdir(path):
Expand Down Expand Up @@ -230,6 +236,12 @@ def upload_string_as_file_body(self, body, bucket, key, kms_key=None):
str: The S3 URI of the uploaded file.
The URI format is: ``s3://{bucket name}/{key}``.
"""
LOGGER.warning(
"'upload_string_as_file_body' method will be deprecated in favor of 'S3Uploader' class "
"(https://sagemaker.readthedocs.io/en/stable/s3.html#sagemaker.s3.S3Uploader) "
"in SageMaker Python SDK v2."
)

if self.s3_resource is None:
s3 = self.boto_session.resource("s3", region_name=self.boto_region_name)
else:
Expand Down Expand Up @@ -3323,6 +3335,7 @@ def get_execution_role(sagemaker_session=None):
Returns:
(str): The role ARN
"""

if not sagemaker_session:
sagemaker_session = Session()
arn = sagemaker_session.get_caller_identity_arn()
Expand Down
5 changes: 4 additions & 1 deletion src/sagemaker/tensorflow/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ def __init__(
)

if distributions is not None:
logger.warning(fw.parameter_v2_rename_warning("distribution", distributions))
train_instance_type = kwargs.get("train_instance_type")
fw.warn_if_parameter_server_with_multi_gpu(
training_instance_type=train_instance_type, distributions=distributions
Expand Down Expand Up @@ -385,7 +386,9 @@ def _validate_args(

if (not self._script_mode_enabled()) and self._only_script_mode_supported():
logger.warning(
"Legacy mode is deprecated in versions 1.13 and higher. Using script mode instead."
"Legacy mode is deprecated in versions 1.13 and higher. Using script mode instead. "
"Legacy mode and its training parameters will be deprecated in "
"SageMaker Python SDK v2. Please use TF 1.13 or higher and script mode."
)
self.script_mode = True

Expand Down
9 changes: 9 additions & 0 deletions tests/unit/test_amazon_estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,3 +486,12 @@ def test_is_latest_xgboost_version():
assert _is_latest_xgboost_version("0.90-1-cpu-py3") is False

assert _is_latest_xgboost_version(XGBOOST_LATEST_VERSION) is True


def test_get_image_uri_warn(caplog):
warning_message = (
"'get_image_uri' method will be deprecated in favor of 'ImageURIProvider' class "
"in SageMaker Python SDK v2."
)
get_image_uri("us-west-2", "kmeans", "latest")
assert warning_message in caplog.text
7 changes: 6 additions & 1 deletion tests/unit/test_inputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from sagemaker.inputs import FileSystemInput


def test_s3_input_all_defaults():
def test_s3_input_all_defaults(caplog):
prefix = "pre"
actual = s3_input(s3_data=prefix)
expected = {
Expand All @@ -32,6 +32,11 @@ def test_s3_input_all_defaults():
}
assert actual.config == expected

warning_message = (
"'s3_input' class will be renamed to 'TrainingInput' in SageMaker Python SDK v2."
)
assert warning_message in caplog.text


def test_s3_input_all_arguments():
prefix = "pre"
Expand Down
8 changes: 7 additions & 1 deletion tests/unit/test_kmeans.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def test_init_required_named(sagemaker_session):
assert kmeans.k == ALL_REQ_ARGS["k"]


def test_all_hyperparameters(sagemaker_session):
def test_all_hyperparameters(sagemaker_session, caplog):
kmeans = KMeans(
sagemaker_session=sagemaker_session,
init_method="random",
Expand Down Expand Up @@ -110,6 +110,12 @@ def test_all_hyperparameters(sagemaker_session):
force_dense="True",
)

warning_message = (
"Parameter 'eval_metrics' hyperparameter will be deprecated for 1P estimators "
"in SageMaker Python SDK v2."
)
assert warning_message in caplog.text


def test_image(sagemaker_session):
kmeans = KMeans(sagemaker_session=sagemaker_session, **ALL_REQ_ARGS)
Expand Down
Loading