Skip to content

breaking: deprecate constants from defaults #1590

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 4 commits into from
Jun 15, 2020
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
8 changes: 0 additions & 8 deletions src/sagemaker/chainer/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,4 @@
"""Placeholder docstring"""
from __future__ import absolute_import

CHAINER_VERSION = "4.1.0"
"""Default Chainer version for when the framework version is not specified.
This is no longer updated so as to not break existing workflows.
"""

LATEST_VERSION = "5.0.0"
"""The latest version of Chainer included in the SageMaker pre-built Docker images."""

LATEST_PY2_VERSION = "5.0.0"
2 changes: 0 additions & 2 deletions src/sagemaker/chainer/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ class Chainer(Framework):
_process_slots_per_host = "sagemaker_process_slots_per_host"
_additional_mpi_options = "sagemaker_additional_mpi_options"

LATEST_VERSION = defaults.LATEST_VERSION

def __init__(
self,
entry_point,
Expand Down
7 changes: 4 additions & 3 deletions src/sagemaker/cli/mxnet.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
from __future__ import absolute_import

from sagemaker.cli.common import HostCommand, TrainCommand
from sagemaker.mxnet import defaults

MXNET_VERSION = "1.2"


def train(args):
Expand Down Expand Up @@ -42,7 +43,7 @@ def create_estimator(self):

return MXNet(
entry_point=self.script,
framework_version=defaults.MXNET_VERSION,
framework_version=MXNET_VERSION,
py_version=self.python,
role=self.role_name,
base_job_name=self.job_name,
Expand All @@ -66,7 +67,7 @@ def create_model(self, model_url):
model_data=model_url,
role=self.role_name,
entry_point=self.script,
framework_version=defaults.MXNET_VERSION,
framework_version=MXNET_VERSION,
py_version=self.python,
name=self.endpoint_name,
env=self.environment,
Expand Down
8 changes: 0 additions & 8 deletions src/sagemaker/mxnet/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,4 @@
"""Placeholder docstring"""
from __future__ import absolute_import

MXNET_VERSION = "1.2"
"""Default MXNet version for when the framework version is not specified.
This is no longer updated so as to not break existing workflows.
"""

LATEST_VERSION = "1.6.0"
"""The latest version of MXNet included in the SageMaker pre-built Docker images."""

LATEST_PY2_VERSION = "1.6.0"
4 changes: 1 addition & 3 deletions src/sagemaker/mxnet/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ class MXNet(Framework):
__framework_name__ = "mxnet"
_LOWEST_SCRIPT_MODE_VERSION = ["1", "3"]

LATEST_VERSION = defaults.LATEST_VERSION

def __init__(
self,
entry_point,
Expand Down Expand Up @@ -115,7 +113,7 @@ def __init__(
:class:`~sagemaker.estimator.EstimatorBase`.
"""
validate_version_or_image_args(framework_version, py_version, image_name)
if py_version and py_version == "py2":
if py_version == "py2":
logger.warning(
python_deprecation_warning(self.__framework_name__, defaults.LATEST_PY2_VERSION)
)
Expand Down
2 changes: 1 addition & 1 deletion src/sagemaker/mxnet/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def __init__(
:class:`~sagemaker.model.Model`.
"""
validate_version_or_image_args(framework_version, py_version, image)
if py_version and py_version == "py2":
if py_version == "py2":
logger.warning(
python_deprecation_warning(self.__framework_name__, defaults.LATEST_PY2_VERSION)
)
Expand Down
10 changes: 0 additions & 10 deletions src/sagemaker/pytorch/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,4 @@
"""Placeholder docstring"""
from __future__ import absolute_import

PYTORCH_VERSION = "0.4"
"""Default PyTorch version for when the framework version is not specified.
The default version is no longer updated so as to not break existing workflows.
"""

LATEST_VERSION = "1.5.0"
"""The latest version of PyTorch included in the SageMaker pre-built Docker images."""

PYTHON_VERSION = "py3"

LATEST_PY2_VERSION = "1.3.1"
2 changes: 0 additions & 2 deletions src/sagemaker/pytorch/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ class PyTorch(Framework):

__framework_name__ = "pytorch"

LATEST_VERSION = defaults.LATEST_VERSION

def __init__(
self,
entry_point,
Expand Down
2 changes: 1 addition & 1 deletion src/sagemaker/pytorch/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def __init__(
:class:`~sagemaker.model.Model`.
"""
validate_version_or_image_args(framework_version, py_version, image)
if py_version and py_version == "py2":
if py_version == "py2":
logger.warning(
python_deprecation_warning(self.__framework_name__, defaults.LATEST_PY2_VERSION)
)
Expand Down
4 changes: 0 additions & 4 deletions src/sagemaker/sklearn/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,3 @@
from __future__ import absolute_import

SKLEARN_NAME = "scikit-learn"

SKLEARN_VERSION = "0.20.0"

LATEST_PY2_VERSION = "0.20.0"
11 changes: 0 additions & 11 deletions src/sagemaker/tensorflow/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,4 @@
"""Placeholder docstring"""
from __future__ import absolute_import

TF_VERSION = "1.11"
"""Default TF version for when the framework version is not specified.
This is no longer updated so as to not break existing workflows.
"""

LATEST_VERSION = "2.2.0"
"""The latest version of TensorFlow included in the SageMaker pre-built Docker images."""

LATEST_SERVING_VERSION = "2.1.0"
"""The latest version of TensorFlow Serving included in the SageMaker pre-built Docker images."""

LATEST_PY2_VERSION = "2.1.0"
4 changes: 0 additions & 4 deletions src/sagemaker/tensorflow/estimator.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ class TensorFlow(Framework):
__framework_name__ = "tensorflow"
_ECR_REPO_NAME = "tensorflow-scriptmode"

LATEST_VERSION = defaults.LATEST_VERSION

_LATEST_1X_VERSION = "1.15.2"

_HIGHEST_LEGACY_MODE_ONLY_VERSION = version.Version("1.10.0")
_HIGHEST_PYTHON_2_VERSION = version.Version("2.1.0")

Expand Down
7 changes: 6 additions & 1 deletion src/sagemaker/xgboost/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,19 @@

XGBOOST_NAME = "xgboost"
XGBOOST_1P_VERSIONS = ["1", "latest"]
XGBOOST_VERSION_0_90 = "0.90"

XGBOOST_VERSION_0_90_1 = "0.90-1"
XGBOOST_VERSION_0_90_2 = "0.90-2"

XGBOOST_LATEST_VERSION = "1.0-1"

# XGBOOST_SUPPORTED_VERSIONS has XGBoost Framework versions sorted from oldest to latest
XGBOOST_SUPPORTED_VERSIONS = [
XGBOOST_VERSION_0_90_1,
XGBOOST_VERSION_0_90_2,
XGBOOST_LATEST_VERSION,
]

# TODO: evaluate use of this constant. it's used in precisely one place in different a module
# may possibly be unnecessary indirection
XGBOOST_VERSION_EQUIVALENTS = ["-cpu-py3"]
49 changes: 20 additions & 29 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,8 @@
from botocore.config import Config

from sagemaker import Session, utils
from sagemaker.chainer import Chainer
from sagemaker.local import LocalSession
from sagemaker.mxnet import MXNet
from sagemaker.pytorch import PyTorch
from sagemaker.rl import RLEstimator
from sagemaker.sklearn.defaults import SKLEARN_VERSION
from sagemaker.tensorflow import TensorFlow
from sagemaker.tensorflow.defaults import LATEST_VERSION, LATEST_SERVING_VERSION

DEFAULT_REGION = "us-west-2"
CUSTOM_BUCKET_NAME_PREFIX = "sagemaker-custom-bucket"
Expand All @@ -49,10 +43,10 @@ def pytest_addoption(parser):
parser.addoption("--sagemaker-client-config", action="store", default=None)
parser.addoption("--sagemaker-runtime-config", action="store", default=None)
parser.addoption("--boto-config", action="store", default=None)
parser.addoption("--chainer-full-version", action="store", default=Chainer.LATEST_VERSION)
parser.addoption("--mxnet-full-version", action="store", default=MXNet.LATEST_VERSION)
parser.addoption("--chainer-full-version", action="store", default="5.0.0")
parser.addoption("--mxnet-full-version", action="store", default="1.6.0")
parser.addoption("--ei-mxnet-full-version", action="store", default="1.5.1")
parser.addoption("--pytorch-full-version", action="store", default=PyTorch.LATEST_VERSION)
parser.addoption("--pytorch-full-version", action="store", default="1.5.0")
parser.addoption(
"--rl-coach-mxnet-full-version",
action="store",
Expand All @@ -64,10 +58,10 @@ def pytest_addoption(parser):
parser.addoption(
"--rl-ray-full-version", action="store", default=RLEstimator.RAY_LATEST_VERSION
)
parser.addoption("--sklearn-full-version", action="store", default=SKLEARN_VERSION)
parser.addoption("--tf-full-version", action="store")
parser.addoption("--sklearn-full-version", action="store", default="0.20.0")
parser.addoption("--tf-full-version", action="store", default="2.2.0")
parser.addoption("--ei-tf-full-version", action="store")
parser.addoption("--xgboost-full-version", action="store", default=SKLEARN_VERSION)
parser.addoption("--xgboost-full-version", action="store", default="1.0-1")


def pytest_configure(config):
Expand Down Expand Up @@ -291,17 +285,13 @@ def sklearn_full_version(request):
return request.config.getoption("--sklearn-full-version")


@pytest.fixture(scope="module", params=[TensorFlow._LATEST_1X_VERSION, LATEST_VERSION])
@pytest.fixture(scope="module")
def tf_full_version(request):
tf_version = request.config.getoption("--tf-full-version")
if tf_version is None:
return request.param
else:
return tf_version
return request.config.getoption("--tf-full-version")


@pytest.fixture(scope="module")
def tf_full_py_version(tf_full_version, request):
def tf_full_py_version(tf_full_version):
"""fixture to match tf_full_version

Fixture exists as such, since tf_full_version may be overridden --tf-full-version.
Expand All @@ -312,9 +302,17 @@ def tf_full_py_version(tf_full_version, request):
version = [int(val) for val in tf_full_version.split(".")]
if version < [1, 11]:
return "py2"
if tf_full_version in [TensorFlow._LATEST_1X_VERSION, LATEST_VERSION]:
return "py37"
return "py3"
if version < [2, 2]:
return "py3"
return "py37"


@pytest.fixture(scope="module")
def tf_serving_version(tf_full_version):
full_version = [int(val) for val in tf_full_version.split(".")]
if full_version < [2, 2]:
return tf_full_version
return "2.1.0"


@pytest.fixture(scope="module", params=["1.15.0", "2.0.0"])
Expand Down Expand Up @@ -384,10 +382,3 @@ def pytest_generate_tests(metafunc):
@pytest.fixture(scope="module")
def xgboost_full_version(request):
return request.config.getoption("--xgboost-full-version")


@pytest.fixture(scope="module")
def tf_serving_version(tf_full_version):
if tf_full_version == LATEST_VERSION:
return LATEST_SERVING_VERSION
return tf_full_version
15 changes: 9 additions & 6 deletions tests/integ/test_airflow_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
from sagemaker.pytorch.estimator import PyTorch
from sagemaker.sklearn import SKLearn
from sagemaker.tensorflow import TensorFlow
from sagemaker.xgboost.defaults import XGBOOST_LATEST_VERSION
from sagemaker.workflow import airflow as sm_airflow
from sagemaker.utils import sagemaker_timestamp

Expand Down Expand Up @@ -550,7 +549,9 @@ def test_sklearn_airflow_config_uploads_data_source_to_s3(


@pytest.mark.canary_quick
def test_tf_airflow_config_uploads_data_source_to_s3(sagemaker_session, cpu_instance_type):
def test_tf_airflow_config_uploads_data_source_to_s3(
sagemaker_session, cpu_instance_type, tf_full_version, tf_full_py_version
):
with timeout(seconds=AIRFLOW_CONFIG_TIMEOUT_IN_SECONDS):
tf = TensorFlow(
image_name=get_image_uri(
Expand All @@ -561,8 +562,8 @@ def test_tf_airflow_config_uploads_data_source_to_s3(sagemaker_session, cpu_inst
train_instance_count=SINGLE_INSTANCE_COUNT,
train_instance_type=cpu_instance_type,
sagemaker_session=sagemaker_session,
framework_version=TensorFlow.LATEST_VERSION,
py_version="py37", # only version available with 2.2.0
framework_version=tf_full_version,
py_version=tf_full_py_version,
metric_definitions=[
{"Name": "train:global_steps", "Regex": r"global_step\/sec:\s(.*)"}
],
Expand All @@ -583,11 +584,13 @@ def test_tf_airflow_config_uploads_data_source_to_s3(sagemaker_session, cpu_inst

@pytest.mark.canary_quick
@pytest.mark.skipif(PYTHON_VERSION == "py2", reason="XGBoost container does not support Python 2.")
def test_xgboost_airflow_config_uploads_data_source_to_s3(sagemaker_session, cpu_instance_type):
def test_xgboost_airflow_config_uploads_data_source_to_s3(
sagemaker_session, cpu_instance_type, xgboost_full_version
):
with timeout(seconds=AIRFLOW_CONFIG_TIMEOUT_IN_SECONDS):
xgboost = XGBoost(
entry_point=os.path.join(DATA_DIR, "dummy_script.py"),
framework_version=XGBOOST_LATEST_VERSION,
framework_version=xgboost_full_version,
role=ROLE,
sagemaker_session=sagemaker_session,
train_instance_type=cpu_instance_type,
Expand Down
8 changes: 4 additions & 4 deletions tests/integ/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def test_github(sagemaker_local_session):

@pytest.mark.local_mode
@pytest.mark.skip("needs a secure authentication approach")
def test_private_github(sagemaker_local_session):
def test_private_github(sagemaker_local_session, mxnet_full_version):
script_path = "mnist.py"
data_path = os.path.join(DATA_DIR, "mxnet_mnist")
git_config = {
Expand All @@ -102,7 +102,7 @@ def test_private_github(sagemaker_local_session):
role="SageMakerRole",
source_dir=source_dir,
dependencies=dependencies,
framework_version=MXNet.LATEST_VERSION,
framework_version=mxnet_full_version,
py_version=PYTHON_VERSION,
train_instance_count=1,
train_instance_type="local",
Expand Down Expand Up @@ -222,7 +222,7 @@ def test_github_with_ssh_passphrase_not_configured(sagemaker_local_session, skle

@pytest.mark.local_mode
@pytest.mark.skip("needs a secure authentication approach")
def test_codecommit(sagemaker_local_session):
def test_codecommit(sagemaker_local_session, mxnet_full_version):
script_path = "mnist.py"
data_path = os.path.join(DATA_DIR, "mxnet_mnist")
git_config = {
Expand All @@ -238,7 +238,7 @@ def test_codecommit(sagemaker_local_session):
role="SageMakerRole",
source_dir=source_dir,
dependencies=dependencies,
framework_version=MXNet.LATEST_VERSION,
framework_version=mxnet_full_version,
py_version=PYTHON_VERSION,
train_instance_count=1,
train_instance_type="local",
Expand Down
8 changes: 5 additions & 3 deletions tests/integ/test_sklearn_train.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import pytest
import numpy

from sagemaker.sklearn.defaults import SKLEARN_VERSION
from sagemaker.sklearn import SKLearn
from sagemaker.sklearn import SKLearnModel
from sagemaker.utils import sagemaker_timestamp, unique_name_from_base
Expand Down Expand Up @@ -147,12 +146,15 @@ def test_deploy_model(
reason="This test has always failed, but the failure was masked by a bug. "
"This test should be fixed. Details in https://github.com/aws/sagemaker-python-sdk/pull/968"
)
def test_async_fit(sagemaker_session, cpu_instance_type):
def test_async_fit(sagemaker_session, cpu_instance_type, sklearn_full_version):
endpoint_name = "test-sklearn-attach-deploy-{}".format(sagemaker_timestamp())

with timeout(minutes=5):
training_job_name = _run_mnist_training_job(
sagemaker_session, cpu_instance_type, sklearn_full_version=SKLEARN_VERSION, wait=False
sagemaker_session,
cpu_instance_type,
sklearn_full_version=sklearn_full_version,
wait=False,
)

print("Waiting to re-attach to the training job: %s" % training_job_name)
Expand Down
Loading