Skip to content

Removed old link and incorrect information from estimator.py docstring #2918

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 9 commits into from
Closed

Removed old link and incorrect information from estimator.py docstring #2918

wants to merge 9 commits into from

Conversation

@judyheflin
Copy link

@judyheflin judyheflin commented Feb 9, 2022

Description of changes: Changed MXNet estimator.py doctstring. Standardized formatting. Removed dead link. Removed information about framework_version and py_version being required unless an image_uri is provided. Customers ran into errors where the framework_version is needed even though they provided an image_uri.

Remaining Questions:

  • If the None default value causes an error, shouldn't we change the default value or remove it entirely?
  • Does it cause an error when you allow the default None value for the py_version as well?
  • Does providing a version number that is different from the exact version provided in the container image cause an error?
  • How can a customer easily find the exact version of Python or MXNet that is provided in their container image?

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Feb 9, 2022

AWS CodeBuild CI Report

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

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Feb 9, 2022

AWS CodeBuild CI Report

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

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Feb 9, 2022

AWS CodeBuild CI Report

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

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

@shreyapandit
Copy link
Contributor

shreyapandit commented Mar 2, 2022

Hi @judyheflin please setup the Git Hooks as mentioned in README and fix the unit tests by running them locally. The sagemaker-bot comments above will guide you for the PR failures.

Have we addressed all remaining open questions and is this ready for review or still WIP?

qidewenwhen and others added 4 commits March 2, 2022 17:17
@qidewenwhen @bencrabtree
@navinsoni @jeniyat
change: reorganize test files for workflow (aws#2960)
a39b750 
Co-authored-by: Ben Crabtree <bencrab@amazon.com>
Co-authored-by: Navin Soni <navinsoni89@gmail.com>
Co-authored-by: Jeniya Tabassum <jeniya.tabassum@gmail.com>
Co-authored-by: Dewen Qi <qidewen@amazon.com>
@Qingzi-Lan @bencrabtree
@navinsoni @jeniyat
feature: TensorFlow 2.4 for Neo (aws#2861)
28fd737 
Co-authored-by: Ben Crabtree <bencrab@amazon.com>
Co-authored-by: Navin Soni <navinsoni89@gmail.com>
Co-authored-by: Jeniya Tabassum <jeniya.tabassum@gmail.com>
@staubhp
fix: Remove sagemaker_job_name from hyperparameters in TrainingStep (a…
20df3d7 
…ws#2950)

Co-authored-by: Payton Staub <pstaub@amazon.com>
@jeniyat
fix: Style update in DataSerializer (aws#2962)
b9f90dc
@judyheflin
Copy link
Author

judyheflin commented Mar 4, 2022

Hi @judyheflin please setup the Git Hooks as mentioned in README and fix the unit tests by running them locally. The sagemaker-bot comments above will guide you for the PR failures.

Have we addressed all remaining open questions and is this ready for review or still WIP?

Still waiting on a response to these questions. @amzahsa said he was taking a look, but not sure if he's still OOO.

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Mar 4, 2022

AWS CodeBuild CI Report

  • CodeBuild project: sagemaker-python-sdk-unit-tests
  • Commit ID: 524b400
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Mar 5, 2022

AWS CodeBuild CI Report

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

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Mar 5, 2022

AWS CodeBuild CI Report

  • CodeBuild project: sagemaker-python-sdk-slow-tests
  • Commit ID: 524b400
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mufaddal-rohawala mufaddal-rohawala changed the base branch from dev to master March 18, 2022 00:56
@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Mar 18, 2022

AWS CodeBuild CI Report

  • CodeBuild project: sagemaker-python-sdk-unit-tests
  • Commit ID: 23b2f92
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Mar 18, 2022

AWS CodeBuild CI Report

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

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

@sagemaker-bot
Copy link
Collaborator

sagemaker-bot commented Mar 18, 2022

AWS CodeBuild CI Report

  • CodeBuild project: sagemaker-python-sdk-slow-tests
  • Commit ID: 23b2f92
  • Result: FAILED
  • Build Logs (available for 30 days)

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

ahsan-z-khan
ahsan-z-khan suggested changes Mar 21, 2022
View reviewed changes
Copy link
Contributor

@ahsan-z-khan ahsan-z-khan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

black-check failed. Can you do a tox -e black-format and commit?

@vikas0203 vikas0203 requested review from a team, mufaddal-rohawala and vikas0203 and removed request for a team November 23, 2022 21:35
@vikas0203 vikas0203 self-assigned this Nov 23, 2022
@vikas0203
Copy link
Contributor

vikas0203 commented Dec 1, 2022

@judyheflin Please resolve merge conflicts.

@mufaddal-rohawala mufaddal-rohawala requested a review from a team as a code owner December 20, 2022 00:31
@knikure knikure removed the request for review from vikas0203 May 1, 2023 23:21
@mufaddal-rohawala
Copy link
Member

mufaddal-rohawala commented May 9, 2023

thanks for contributing but this code change has been pending for too long. Please reopen if this change is still relevant!

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

Reviewers

@mufaddal-rohawala mufaddal-rohawala Awaiting requested review from mufaddal-rohawala mufaddal-rohawala was automatically assigned from aws/sagemaker-ml-frameworks

1 more reviewer

@ahsan-z-khan ahsan-z-khan ahsan-z-khan requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mufaddal-rohawala mufaddal-rohawala

Labels

needs contributor changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.