Update empty framework_version warning (#472)

This is in accordance with our new strategy around
not making framework_version completely mandatory.

Author: laurenyu

CHANGELOG.rst

@@ -12,6 +12,7 @@ CHANGELOG
 * enhancement: Local Mode: support optional input channels
 * build: added pylint
 * build: upgrade docker-compose to 1.23
+* enhancement: Frameworks: update warning for not setting framework_version as we aren't planning a breaking change anymore
 
 1.14.1
 ======

src/sagemaker/chainer/estimator.py

@@ -82,7 +82,7 @@ def __init__(self, entry_point, use_mpi=None, num_processes=None, process_slots_
             **kwargs: Additional kwargs passed to the :class:`~sagemaker.estimator.Framework` constructor.
         """
         if framework_version is None:
-            logger.warning(empty_framework_version_warning(CHAINER_VERSION))
+            logger.warning(empty_framework_version_warning(CHAINER_VERSION, CHAINER_VERSION))
         self.framework_version = framework_version or CHAINER_VERSION
 
         super(Chainer, self).__init__(entry_point, source_dir, hyperparameters,

src/sagemaker/fw_utils.py

@@ -26,10 +26,10 @@
 instantiated with positional or keyword arguments.
 """
 
-EMPTY_FRAMEWORK_VERSION_WARNING = 'In an upcoming version of the SageMaker Python SDK, ' \
-                                  'framework_version will be required to create an estimator. ' \
-                                  'Please add framework_version={} to your constructor to avoid ' \
-                                  'an error in the future.'
+EMPTY_FRAMEWORK_VERSION_WARNING = 'No framework_version specified, defaulting to version {}.'
+LATER_FRAMEWORK_VERSION_WARNING = 'This is not the latest supported version. ' \
+                                  'If you would like to use version {latest}, ' \
+                                  'please add framework_version={latest} to your constructor.'
 
 VALID_PY_VERSIONS = ['py2', 'py3']
 
@@ -232,5 +232,8 @@ def model_code_key_prefix(code_location_key_prefix, model_name, image):
     return '/'.join(filter(None, [code_location_key_prefix, model_name or training_job_name]))
 
 
-def empty_framework_version_warning(default_version):
-    return EMPTY_FRAMEWORK_VERSION_WARNING.format(default_version)
+def empty_framework_version_warning(default_version, latest_version):
+    msgs = [EMPTY_FRAMEWORK_VERSION_WARNING.format(default_version)]
+    if default_version != latest_version:
+        msgs.append(LATER_FRAMEWORK_VERSION_WARNING.format(latest=latest_version))
+    return ' '.join(msgs)

src/sagemaker/mxnet/estimator.py

@@ -31,6 +31,7 @@ class MXNet(Framework):
 
     _LOWEST_SCRIPT_MODE_VERSION = ['1', '3']
     LAUNCH_PS_ENV_NAME = 'sagemaker_parameter_server_enabled'
+    LATEST_VERSION = '1.3'
 
     def __init__(self, entry_point, source_dir=None, hyperparameters=None, py_version='py2',
                  framework_version=None, image_name=None, distributions=None, **kwargs):
@@ -72,7 +73,7 @@ def __init__(self, entry_point, source_dir=None, hyperparameters=None, py_versio
             **kwargs: Additional kwargs passed to the :class:`~sagemaker.estimator.Framework` constructor.
         """
         if framework_version is None:
-            logger.warning(empty_framework_version_warning(MXNET_VERSION))
+            logger.warning(empty_framework_version_warning(MXNET_VERSION, self.LATEST_VERSION))
         self.framework_version = framework_version or MXNET_VERSION
 
         super(MXNet, self).__init__(entry_point, source_dir, hyperparameters,

src/sagemaker/pytorch/estimator.py

@@ -67,7 +67,7 @@ def __init__(self, entry_point, source_dir=None, hyperparameters=None, py_versio
             **kwargs: Additional kwargs passed to the :class:`~sagemaker.estimator.Framework` constructor.
         """
         if framework_version is None:
-            logger.warning(empty_framework_version_warning(PYTORCH_VERSION))
+            logger.warning(empty_framework_version_warning(PYTORCH_VERSION, PYTORCH_VERSION))
         self.framework_version = framework_version or PYTORCH_VERSION
 
         super(PyTorch, self).__init__(entry_point, source_dir, hyperparameters, image_name=image_name, **kwargs)

src/sagemaker/tensorflow/estimator.py

@@ -164,8 +164,8 @@ class TensorFlow(Framework):
     __framework_name__ = 'tensorflow'
 
     def __init__(self, training_steps=None, evaluation_steps=None, checkpoint_path=None,
-                 py_version='py2',
-                 framework_version=None, requirements_file='', image_name=None, **kwargs):
+                 py_version='py2', framework_version=None, requirements_file='', image_name=None,
+                 **kwargs):
         """Initialize an ``TensorFlow`` estimator.
         Args:
             training_steps (int): Perform this many steps of training. `None`, the default means train forever.
@@ -188,7 +188,7 @@ def __init__(self, training_steps=None, evaluation_steps=None, checkpoint_path=N
             **kwargs: Additional kwargs passed to the Framework constructor.
         """
         if framework_version is None:
-            LOGGER.warning(empty_framework_version_warning(TF_VERSION))
+            LOGGER.warning(empty_framework_version_warning(TF_VERSION, TF_VERSION))
         self.framework_version = framework_version or TF_VERSION
 
         super(TensorFlow, self).__init__(image_name=image_name, **kwargs)

tests/unit/test_chainer.py

@@ -459,3 +459,13 @@ def test_attach_custom_image(sagemaker_session):
     estimator = Chainer.attach(training_job_name='neo', sagemaker_session=sagemaker_session)
     assert estimator.image_name == training_image
     assert estimator.train_image() == training_image
+
+
+@patch('sagemaker.chainer.estimator.empty_framework_version_warning')
+def test_empty_framework_version(warning, sagemaker_session):
+    estimator = Chainer(entry_point=SCRIPT_PATH, role=ROLE, sagemaker_session=sagemaker_session,
+                        train_instance_count=INSTANCE_COUNT, train_instance_type=INSTANCE_TYPE,
+                        framework_version=None)
+
+    assert estimator.framework_version == defaults.CHAINER_VERSION
+    warning.assert_called_with(defaults.CHAINER_VERSION, defaults.CHAINER_VERSION)

tests/unit/test_mxnet.py

@@ -395,3 +395,13 @@ def test_estimator_wrong_version_launch_parameter_server(sagemaker_session):
               train_instance_count=INSTANCE_COUNT, train_instance_type=INSTANCE_TYPE,
               distributions=LAUNCH_PS_DISTRIBUTIONS_DICT, framework_version='1.2.1')
     assert 'The distributions option is valid for only versions 1.3 and higher' in str(e)
+
+
+@patch('sagemaker.mxnet.estimator.empty_framework_version_warning')
+def test_empty_framework_version(warning, sagemaker_session):
+    mx = MXNet(entry_point=SCRIPT_PATH, role=ROLE, sagemaker_session=sagemaker_session,
+               train_instance_count=INSTANCE_COUNT, train_instance_type=INSTANCE_TYPE,
+               framework_version=None)
+
+    assert mx.framework_version == defaults.MXNET_VERSION
+    warning.assert_called_with(defaults.MXNET_VERSION, mx.LATEST_VERSION)

tests/unit/test_pytorch.py

@@ -362,3 +362,13 @@ def test_attach_custom_image(sagemaker_session):
     assert estimator.latest_training_job.job_name == 'neo'
     assert estimator.image_name == training_image
     assert estimator.train_image() == training_image
+
+
+@patch('sagemaker.pytorch.estimator.empty_framework_version_warning')
+def test_empty_framework_version(warning, sagemaker_session):
+    estimator = PyTorch(entry_point=SCRIPT_PATH, role=ROLE, sagemaker_session=sagemaker_session,
+                        train_instance_count=INSTANCE_COUNT, train_instance_type=INSTANCE_TYPE,
+                        framework_version=None)
+
+    assert estimator.framework_version == defaults.PYTORCH_VERSION
+    warning.assert_called_with(defaults.PYTORCH_VERSION, defaults.PYTORCH_VERSION)

tests/unit/test_tf_estimator.py

@@ -634,3 +634,13 @@ def test_attach_custom_image(sagemaker_session):
     estimator = TensorFlow.attach(training_job_name='neo', sagemaker_session=sagemaker_session)
     assert estimator.image_name == training_image
     assert estimator.train_image() == training_image
+
+
+@patch('sagemaker.tensorflow.estimator.empty_framework_version_warning')
+def test_empty_framework_version(warning, sagemaker_session):
+    estimator = TensorFlow(entry_point=SCRIPT_PATH, role=ROLE, sagemaker_session=sagemaker_session,
+                           train_instance_count=INSTANCE_COUNT, train_instance_type=INSTANCE_TYPE,
+                           framework_version=None)
+
+    assert estimator.framework_version == defaults.TF_VERSION
+    warning.assert_called_with(defaults.TF_VERSION, defaults.TF_VERSION)