change: add modifier for s3_input class #1699

chuyang-deng · 2020-07-10T07:09:09Z

Issue #, if available:
breaking changes brought in #1680

Description of changes:
Add s3_input modifier to v2 cli.
Convert:

sagemaker.s3_input --> sagemaker.TrainingInput
s3_input --> TrainingInput

Testing done:
Unit test

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

I have read the CONTRIBUTING doc
I used the commit message format described in CONTRIBUTING
I have passed the region in to all S3 and STS clients that I've initialized as part of this change.
I have updated any necessary documentation, including READMEs and API docs (if appropriate)

Tests

I have added tests that prove my fix is effective or that my feature works (if appropriate)
I have checked that my tests are not configured for a specific region or account (if appropriate)
I have used unique_name_from_base to create resource names in integ tests (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…thon-sdk into rename-s3-input

sagemaker-bot · 2020-07-10T07:16:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: e1a0eed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T07:16:19Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 083da46
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T07:35:45Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 083da46
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T07:37:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: e1a0eed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T07:44:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: 083da46
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T07:49:27Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 083da46
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T07:50:06Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: e1a0eed
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-07-10T15:56:34Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+"""Classes to modify Predictor code to be compatible
+with version 2.0 and later of the SageMaker Python SDK.


don't forget to update this

laurenyu · 2020-07-10T16:08:58Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+
+BASE_S3_INPUT = "s3_input"
+SESSION = "session"
+S3_INPUT = {"s3_input": ("sagemaker", "sagemaker.session")}


also include sagemaker.inputs?

Updated the namespaces and added a test case.

laurenyu · 2020-07-10T16:09:33Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+from sagemaker.cli.compatibility.v2.modifiers.modifier import Modifier
+
+BASE_S3_INPUT = "s3_input"
+SESSION = "session"


this looks to be unused

laurenyu · 2020-07-10T16:10:20Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+from sagemaker.cli.compatibility.v2.modifiers import matching
+from sagemaker.cli.compatibility.v2.modifiers.modifier import Modifier
+
+BASE_S3_INPUT = "s3_input"


maybe S3_INPUT_NAME instead? I don't think "base" gives much info here.

laurenyu · 2020-07-10T16:11:58Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+        Returns:
+            bool: If the import statement imports ``RealTimePredictor`` from the correct module.
+        """
+        return node.module in S3_INPUT[BASE_S3_INPUT] and any(


seeing this, I think it might be better to define the list of namespaces separately. S3_INPUT[BASE_S3_INPUT] is a little unintuitive to read on its own here

Yes. I got rid of the dict and used namespaces to check the module.

laurenyu · 2020-07-10T16:12:12Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+    """A class to update import statements of ``s3_input``."""
+
+    def node_should_be_modified(self, node):
+        """Checks if the import statement imports ``RealTimePredictor`` from the correct module.


don't forget to update the docstring

laurenyu · 2020-07-10T16:12:49Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+        _rename_class(node)
+
+
+def _rename_class(node):


I don't know if you need to make this a standalone function, unless maybe if you're refactoring it so that it can be used elsewhere too

Yeah, there's no need to make it a function. I've moved the logic to modify_node

laurenyu · 2020-07-10T17:26:20Z

also don't forget to update ast_transformer.py

…thon-sdk into rename-s3-input

sagemaker-bot · 2020-07-10T20:58:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 99a9652
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T21:01:18Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: daff7ac
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

tests/unit/sagemaker/cli/compatibility/v2/modifiers/test_training_input.py

laurenyu · 2020-07-10T21:07:41Z

tests/unit/sagemaker/cli/compatibility/v2/modifiers/test_training_input.py

+
+    node = ast_import("from sagemaker.session import s3_input as training_input")
+    modifier.modify_node(node)
+    expected_result = "from sagemaker.session import TrainingInput as training_input"


Suggested change

expected_result = "from sagemaker.session import TrainingInput as training_input"

expected_result = "from sagemaker.inputs import TrainingInput as training_input"

sagemaker-bot · 2020-07-10T21:20:22Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 99a9652
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T21:22:36Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: daff7ac
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T21:25:24Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: daff7ac
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T21:31:59Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 99a9652
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-10T21:33:41Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: daff7ac
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…thon-sdk into rename-s3-input

sagemaker-bot · 2020-07-11T06:47:13Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 13e1f7d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-11T07:07:44Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 13e1f7d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-11T07:09:31Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: 04cdc7b
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-11T07:20:05Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 13e1f7d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-11T07:30:07Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: 04cdc7b
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-11T07:31:14Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: 04cdc7b
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-11T07:42:20Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: 04cdc7b
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

laurenyu · 2020-07-13T15:23:35Z

src/sagemaker/cli/compatibility/v2/modifiers/training_input.py

+        - ``sagemaker.s3_input``
+        - ``sagemaker.session.s3_input``
+        - ``s3_input``


Suggested change

- ``sagemaker.s3_input``

- ``sagemaker.session.s3_input``

- ``s3_input``

- ``sagemaker.s3_input``

- ``sagemaker.inputs.s3_input``

- ``sagemaker.session.s3_input``

- ``s3_input``

laurenyu · 2020-07-13T15:28:38Z

tests/unit/sagemaker/cli/compatibility/v2/modifiers/test_training_input.py

+    node = ast_call("s3_input(s3_data='s3://a')")
+    modifier.modify_node(node)
+    assert "TrainingInput(s3_data='s3://a')" == pasta.dump(node)
+
+    node = ast_call("sagemaker.s3_input(s3_data='s3://a')")
+    modifier.modify_node(node)
+    assert "sagemaker.TrainingInput(s3_data='s3://a')" == pasta.dump(node)
+
+    node = ast_call("session.s3_input(s3_data='s3://a')")
+    modifier.modify_node(node)
+    assert "inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)
+
+    node = ast_call("inputs.s3_input(s3_data='s3://a')")
+    modifier.modify_node(node)
+    assert "inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)
+
+    node = ast_call("sagemaker.inputs.s3_input(s3_data='s3://a')")
+    modifier.modify_node(node)
+    assert "sagemaker.inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)
+
+    node = ast_call("sagemaker.session.s3_input(s3_data='s3://a')")
+    modifier.modify_node(node)
+    assert "sagemaker.inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)


optional - you could also make an iterable of tuples to reduce how much code is repeated

Suggested change

node = ast_call("s3_input(s3_data='s3://a')")

modifier.modify_node(node)

assert "TrainingInput(s3_data='s3://a')" == pasta.dump(node)

node = ast_call("sagemaker.s3_input(s3_data='s3://a')")

modifier.modify_node(node)

assert "sagemaker.TrainingInput(s3_data='s3://a')" == pasta.dump(node)

node = ast_call("session.s3_input(s3_data='s3://a')")

modifier.modify_node(node)

assert "inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)

node = ast_call("inputs.s3_input(s3_data='s3://a')")

modifier.modify_node(node)

assert "inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)

node = ast_call("sagemaker.inputs.s3_input(s3_data='s3://a')")

modifier.modify_node(node)

assert "sagemaker.inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)

node = ast_call("sagemaker.session.s3_input(s3_data='s3://a')")

modifier.modify_node(node)

assert "sagemaker.inputs.TrainingInput(s3_data='s3://a')" == pasta.dump(node)

calls = (

("s3_input(s3_data='s3://a')", TrainingInput(s3_data='s3://a')"),

("sagemaker.s3_input(s3_data='s3://a')", "sagemaker.TrainingInput(s3_data='s3://a')"),

("session.s3_input(s3_data='s3://a')", "inputs.TrainingInput(s3_data='s3://a')"),

("inputs.s3_input(s3_data='s3://a')", "inputs.TrainingInput(s3_data='s3://a')"),

("sagemaker.inputs.s3_input(s3_data='s3://a')", "sagemaker.inputs.TrainingInput(s3_data='s3://a')"),

("sagemaker.session.s3_input(s3_data='s3://a')", "sagemaker.inputs.TrainingInput(s3_data='s3://a')"),

)

for call, expected in calls:

node = ast_call(call)

modifier.modify_node(node)

assert expected == pasta.dump(node)

sagemaker-bot · 2020-07-13T18:10:29Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-unit-tests
Commit ID: de40298
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-13T18:32:28Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-notebook-tests
Commit ID: de40298
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-13T18:42:51Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-local-mode-tests
Commit ID: de40298
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

sagemaker-bot · 2020-07-13T18:54:54Z

AWS CodeBuild CI Report

CodeBuild project: sagemaker-python-sdk-zwei-pr
Commit ID: de40298
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Chuyang Deng and others added 10 commits July 7, 2020 22:30

breaking: rename s3_input to TrainingInput

495e289

Merge branch 'zwei' into rename-s3-input

bc4f20c

Merge branch 'zwei' into rename-s3-input

50ad633

remove TrainingInput import from session

d15f823

Merge branch 'rename-s3-input' of github.com:ChuyangDeng/sagemaker-py…

5f030dd

…thon-sdk into rename-s3-input

update docstring

a6e4830

Merge branch 'zwei' into rename-s3-input

4cd044b

change: add modifier for s3_input class

4c08cd7

Merge branch 'rename-s3-input' of github.com:ChuyangDeng/sagemaker-py…

e1a0eed

…thon-sdk into rename-s3-input

Merge branch 'zwei' into rename-s3-input

083da46

laurenyu reviewed Jul 10, 2020

View reviewed changes

chuyang-deng closed this Jul 10, 2020

chuyang-deng reopened this Jul 10, 2020

Chuyang Deng and others added 3 commits July 10, 2020 13:50

modify namespaces

cfb7c2e

Merge branch 'rename-s3-input' of github.com:ChuyangDeng/sagemaker-py…

99a9652

…thon-sdk into rename-s3-input

Merge branch 'zwei' into rename-s3-input

daff7ac

laurenyu reviewed Jul 10, 2020

View reviewed changes

Chuyang Deng added 2 commits July 10, 2020 23:39

rename namespace

060d716

Merge branch 'rename-s3-input' of github.com:ChuyangDeng/sagemaker-py…

13e1f7d

…thon-sdk into rename-s3-input

Merge branch 'zwei' into rename-s3-input

04cdc7b

laurenyu approved these changes Jul 13, 2020

View reviewed changes

Merge branch 'zwei' into rename-s3-input

de40298

chuyang-deng merged commit 24b2ab9 into aws:zwei Jul 13, 2020

		"""Classes to modify Predictor code to be compatible
		with version 2.0 and later of the SageMaker Python SDK.

	expected_result = "from sagemaker.session import TrainingInput as training_input"
	expected_result = "from sagemaker.inputs import TrainingInput as training_input"

change: add modifier for s3_input class #1699

change: add modifier for s3_input class #1699

Uh oh!

Conversation

chuyang-deng commented Jul 10, 2020

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laurenyu commented Jul 10, 2020

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 10, 2020

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Jul 11, 2020

AWS CodeBuild CI Report

Uh oh!