Skip to content

Conversation

@jld23
Copy link

@jld23 jld23 commented Feb 15, 2023

Issue #, if available:

Description of changes:
The current notebooks are based on sagemaker sdk 2.48 which is 18 months old and ~85 versions behind. I updated the sagemaker version check from:

if sagemaker.__version__ < '2.48.1':
    subprocess.check_call([sys.executable, '-m', 'pip', 'install', 'sagemaker==2.48.1'])
    importlib.reload(sagemaker)

to this because the prior version did a string comparison so 132 < 48.

current_sagemaker_release = '2.132.0'
curr_major, curr_minor, curr_patch = current_sagemaker_release.split('.')
sagemaker_version = sagemaker.__version__
sm_major, sm_minor, sm_patch = sagemaker_version.split('.')
if int(sm_major) < int(curr_major) or int(sm_minor) < int(curr_minor):
    subprocess.check_call([sys.executable, '-m', 'pip', 'install', f'sagemaker=={current_sagemaker_release}'])
    importlib.reload(sagemaker)

I also updated the screen shots and instructions in the ./images folder to reflect the UI changes to SageMaker Studio in November 2022.

I ran all the notebooks with the updated sagemaker python SDK (v2.132.0) and all the notebooks ran successfully except for m6_nb1 for which I'll open a separate issue.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jld23
Copy link
Author

jld23 commented Mar 2, 2023

I reviewed the diffs caused by the latest push. All are markdown cell id values that aren't visible to the user. I'm happy to update the conflict but I'd like to do it once so if you can give me. time line or indication if this pull request will be accepted I would appreciate the communication.

@markproy
Copy link
Contributor

markproy commented Mar 2, 2023

yeah, I'd rather not commit 33 file changes unnecessarily. if you can get it back down to a simple handful of changes, we can get it approved in short order. no rush. once you have it ready, we can probably get someone to get through it w/in a week most likely.

@jld23
Copy link
Author

jld23 commented Mar 16, 2023

@markproy I updated the commit to just the 15 images and 8 notebooks that needed updates. There are no longer any conflicts with the base branch.

@markproy
Copy link
Contributor

Thanks for streamlining the changes. I really appreciate the updated images! The 8 nb's that need to change still have lots of diffs for some reason. Why so many changes for each nb?

@jld23
Copy link
Author

jld23 commented Mar 20, 2023

It looks like the changes are in metadata of the notebook. I used python 3.8 and the previous notebooks were 3.7. Also, it appears Sagemaker is writing a bunch of metadata about instance types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants