Sagemaker-python-sdk: Create a script to aid with migrating from v1.x to v2.0

Created on 8 May 2020  路  9Comments  路  Source: aws/sagemaker-python-sdk

To go along with the breaking changes proposed in https://github.com/aws/sagemaker-python-sdk/issues/1459, we should provide a script that makes it easier to update one's code. For example, TensorFlow did this for upgrading to TF 2.0 (source).

feature request maintenance

Most helpful comment

Hi @laurenyu , I made a PR with the tests here https://github.com/aws/sagemaker-python-sdk/pull/1555 .
Please, let me know if there's any fix I need to add.

All 9 comments

Hi @laurenyu , would love to contribute to this feature. Are external contributors allowed in the migration? If yes, how can I help?

@Olamyy yes! I've just posted a PR (#1492) to start laying some ground work. Take a look, comment with suggestions for refactoring, renaming, etc.

after that, I think the next steps would include:

  • setting up some tests
  • class(es) for reading source files and feeding the code into the ASTTransformer class, and dumping the results into a new file
  • the CLI interface (see TensorFlow's for an example)
  • configuration for vending the CLI tool
  • some of the other options that TF's script has, such as handling directories, printing a report of the changes, etc.

anything of the above jump out as something you'd like to pick up?

I went through the ASTTransformer PR and it seems fine to me.
I want to try and move on to setting up tests.
Should these tests be ASTTransformer related or general v2 migration tests?

@Olamyy just for the ASTTransformer and code related to this migration script

edit: since I've merged some more code, just wanted to clarify - we're looking for some unit tests for testing the stuff in tools/compatibility/v2 that can also be run from buildspec-unittests.yml. You don't have to fill in tests for everything that's been merged this week, though - I can definitely help out with that :)

@Olamyy have you had a chance to try working on things here?

Hi @laurenyu , I currently have a setup here https://github.com/Olamyy/sagemaker-python-sdk/tree/v2-compability-tests/tests/unit/v2 where I plan to write the tests. I've been able to write a simple function to test the output of the ASTTransformer. here. However, i'm honestly not sure what other thing to test. Could you provide some tips on how you'll go about this for me? This would help me decide what tests I need to write and which are not important.

@Olamyy what you've got is good; I didn't have too much in mind at the time because, you're right, there's not a lot of code in ASTTransformer. I think I'd also add a test case where there shouldn't be any change with the code.

The other thing is that we use pytest rather than unittest. I added some tests last week for my more recent changes, so that can help with seeing how some of the unit tests in this repo are written: https://github.com/aws/sagemaker-python-sdk/tree/zwei/tests/unit/sagemaker/cli/compatibility/v2

Hi @laurenyu , I made a PR with the tests here https://github.com/aws/sagemaker-python-sdk/pull/1555 .
Please, let me know if there's any fix I need to add.

as we're nearing the release of v2.0, noting what's left:

Was this page helpful?
0 / 5 - 0 ratings

Related issues

meownoid picture meownoid  路  5Comments

velociraptor111 picture velociraptor111  路  3Comments

velociraptor111 picture velociraptor111  路  3Comments

elvinx picture elvinx  路  4Comments

athewsey picture athewsey  路  4Comments