feature: Data Serializer by jeniyat · Pull Request #2956 · aws/sagemaker-python-sdk

jeniyat · 2022-02-23T20:28:57Z

Issue #, if available:
Includes a serializer for multimodal support

Description of changes:
Introduced DataSerializer class that utilizes current SimpleBaseSerializer class to read and serialize data in different formats, i.e, audio/image

Testing done:

Tested the data serializer with different inputs.
Tested the data serializer with the default content_type="file-path/raw-bytes"
Tested the changes in this PR locally, where it passed all the tests (attached the local output below)

JT:sagemaker-python-sdk jeniyat$ ./.githooks/pre-push
GLOB sdist-make: /Users/jeniyat/Desktop/HuggingFace/source_repo/sagemaker-python-sdk/setup.py
✔ OK black-check in 8.605 seconds
✔ OK twine in 11.449 seconds
✔ OK pylint in 21.92 seconds
✔ OK docstyle in 24.015 seconds
✔ OK flake8 in 41.267 seconds
__________________________________________________________________________________________________________________________________________________________ summary ___________________________________________________________________________________________________________________________________________________________
  flake8: commands succeeded
  pylint: commands succeeded
  docstyle: commands succeeded
  black-check: commands succeeded
  twine: commands succeeded
  congratulations :)
=================== flake8,pylint,docstyle,black-check,twine execution time ===================
44 seconds

GLOB sdist-make: /Users/jeniyat/Desktop/HuggingFace/source_repo/sagemaker-python-sdk/setup.py
✔ OK doc8 in 8.755 seconds
✔ OK sphinx in 4 minutes, 46.133 seconds
__________________________________________________________________________________________________________________________________________________________ summary ___________________________________________________________________________________________________________________________________________________________
  sphinx: commands succeeded
  doc8: commands succeeded
  congratulations :)
=================== sphinx,doc8 execution time ===================
4 minutes and 49 seconds

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 certify that the changes I am introducing will be backword compatible, and I have discussed concerns about this, if any, with the Python SDK team
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 added unit and/or integration tests as appropriate to ensure backward compatibility of the changes
I have tested the tests locally.
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.

sagemaker-bot · 2022-02-23T20:50:08Z

AWS CodeBuild CI Report

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

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

codecov-commenter · 2022-02-23T21:00:52Z

Codecov Report

Merging #2956 (fe0bf46) into dev (4ce6623) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##              dev    #2956      +/-   ##
==========================================
- Coverage   89.82%   89.80%   -0.02%     
==========================================
  Files         196      196              
  Lines       16548    16563      +15     
==========================================
+ Hits        14864    14875      +11     
- Misses       1684     1688       +4

Impacted Files	Coverage Δ
src/sagemaker/serializers.py	`92.96% <80.00%> (-1.73%)`	⬇️
src/sagemaker/session.py	`70.39% <0.00%> (-0.08%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ce6623...fe0bf46. Read the comment docs.

sagemaker-bot · 2022-02-23T21:29:53Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-23T21:35:35Z

AWS CodeBuild CI Report

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

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

navinsoni · 2022-02-23T23:59:59Z

+        if isinstance(data, str):
+            if not os.path.exists(data):
+                raise ValueError(f"{data} is not a valid file path.")
+            image = open(data, "rb")


Please close file here.

Add Exception handling

Addressed in current revision.

navinsoni

Please close file before returning from function

sagemaker-bot · 2022-02-24T02:55:33Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T02:57:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T03:31:06Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T03:31:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T03:37:59Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T03:44:15Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T03:55:26Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T18:31:22Z

AWS CodeBuild CI Report

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

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

mufaddal-rohawala · 2022-02-24T18:36:19Z

+            try:
+                dataFile = open(data, "rb")
+            except Exception:
+                raise ValueError(f"{data} is not a valid file-path.")


Lets generalise the exception and not conclude not a valid file-path for better debugging.

try: dataFile = open(data, "rb") dataFileInfo = dataFile.read() dataFile.close() except Exception as e: raise ValueError(f"Could not open/read file: {data}. {e.message}")

updated in the current revision.

sagemaker-bot · 2022-02-24T19:07:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T19:36:19Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T19:39:14Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T19:44:00Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T20:21:46Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T20:32:08Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T20:46:40Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-24T20:46:40Z

AWS CodeBuild CI Report

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

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

navinsoni · 2022-02-25T00:31:46Z

+                dataFile.close()
+            except Exception as e:
+                raise ValueError(f"Could not open/read file: {data}. {e}")
+            return dataFileInfo


can you please move this in try block

sagemaker-bot · 2022-02-25T04:07:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-25T04:58:41Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-25T05:50:34Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-28T04:04:58Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-28T04:49:23Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-28T04:49:24Z

AWS CodeBuild CI Report

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

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

sagemaker-bot · 2022-02-28T04:55:58Z

AWS CodeBuild CI Report

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

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

philschmid · 2022-03-01T07:59:09Z

+                dataFile = open(data, "rb")
+                dataFileInfo = dataFile.read()
+                dataFile.close()


Wouldn't it have made more sense to use the context manager for reading the file, to be safer about error handling when .read() fails?
Also shouldn't be the naming schema for variables be snake_case rather than camelCase? Similar to all other Serializer

As this PR is already merged, added a new PR with the suggested changes here: #2962

* change: update code to get commit_id in codepipeline (#2961) * feature: Data Serializer (#2956) * change: reorganize test files for workflow (#2960) 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> * feature: TensorFlow 2.4 for Neo (#2861) 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> * fix: Remove sagemaker_job_name from hyperparameters in TrainingStep (#2950) Co-authored-by: Payton Staub <pstaub@amazon.com> * fix: Style update in DataSerializer (#2962) * documentation: smddp doc update (#2968) * fix: container env generation for S3 URI and add test for the same (#2971) * documentation: update sagemaker training compiler docstring (#2969) * feat: Python 3.9 for readthedocs (#2973) 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> Co-authored-by: Payton Staub <pstaub@amazon.com> Co-authored-by: qidewenwhen <32910701+qidewenwhen@users.noreply.github.com> Co-authored-by: Qingzi-Lan <83724147+Qingzi-Lan@users.noreply.github.com> Co-authored-by: Payton Staub <staubhpa@gmail.com> * fix doc structure * archive 1.6.0 doc * add new args, refs, and links * fix version number * incorp eng feedback, update docstrings, improve xref * Trigger Build * minor fix, trigger build again * fix typo Co-authored-by: Navin Soni <navinsoni89@gmail.com> Co-authored-by: Jeniya Tabassum <jeniya.tabassum@gmail.com> Co-authored-by: qidewenwhen <32910701+qidewenwhen@users.noreply.github.com> Co-authored-by: Ben Crabtree <bencrab@amazon.com> Co-authored-by: Dewen Qi <qidewen@amazon.com> Co-authored-by: Qingzi-Lan <83724147+Qingzi-Lan@users.noreply.github.com> Co-authored-by: Payton Staub <staubhpa@gmail.com> Co-authored-by: Payton Staub <pstaub@amazon.com> Co-authored-by: Shreya Pandit <shreya.pandit25@gmail.com> Co-authored-by: Ahsan Khan <ahsan.al.zaki@gmail.com> Co-authored-by: Mufaddal Rohawala <89424143+mufaddal-rohawala@users.noreply.github.com>

* change: update code to get commit_id in codepipeline (aws#2961) * feature: Data Serializer (aws#2956) * change: reorganize test files for workflow (aws#2960) 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> * feature: TensorFlow 2.4 for Neo (aws#2861) 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> * fix: Remove sagemaker_job_name from hyperparameters in TrainingStep (aws#2950) Co-authored-by: Payton Staub <pstaub@amazon.com> * fix: Style update in DataSerializer (aws#2962) * documentation: smddp doc update (aws#2968) * fix: container env generation for S3 URI and add test for the same (aws#2971) * documentation: update sagemaker training compiler docstring (aws#2969) * feat: Python 3.9 for readthedocs (aws#2973) 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> Co-authored-by: Payton Staub <pstaub@amazon.com> Co-authored-by: qidewenwhen <32910701+qidewenwhen@users.noreply.github.com> Co-authored-by: Qingzi-Lan <83724147+Qingzi-Lan@users.noreply.github.com> Co-authored-by: Payton Staub <staubhpa@gmail.com> * fix doc structure * archive 1.6.0 doc * add new args, refs, and links * fix version number * incorp eng feedback, update docstrings, improve xref * Trigger Build * minor fix, trigger build again * fix typo Co-authored-by: Navin Soni <navinsoni89@gmail.com> Co-authored-by: Jeniya Tabassum <jeniya.tabassum@gmail.com> Co-authored-by: qidewenwhen <32910701+qidewenwhen@users.noreply.github.com> Co-authored-by: Ben Crabtree <bencrab@amazon.com> Co-authored-by: Dewen Qi <qidewen@amazon.com> Co-authored-by: Qingzi-Lan <83724147+Qingzi-Lan@users.noreply.github.com> Co-authored-by: Payton Staub <staubhpa@gmail.com> Co-authored-by: Payton Staub <pstaub@amazon.com> Co-authored-by: Shreya Pandit <shreya.pandit25@gmail.com> Co-authored-by: Ahsan Khan <ahsan.al.zaki@gmail.com> Co-authored-by: Mufaddal Rohawala <89424143+mufaddal-rohawala@users.noreply.github.com>

feature: Data Serializer capabile of supporting image audio files

ea7e9a9

jeniyat requested review from mufaddal-rohawala and navinsoni February 23, 2022 23:56

navinsoni reviewed Feb 23, 2022

View reviewed changes

navinsoni suggested changes Feb 24, 2022

View reviewed changes

jeniyat added 2 commits February 23, 2022 19:32

added exception handling

ecb4800

updated doc string

1cb9de6

Merge branch 'aws:dev' into jeniyat/data-serializer

d79af25

mufaddal-rohawala reviewed Feb 24, 2022

View reviewed changes

jeniyat added 3 commits February 24, 2022 12:15

update exception handling

e2936b2

update exception handling

7e4893c

updated doc string

8368584

mufaddal-rohawala approved these changes Feb 24, 2022

View reviewed changes

navinsoni suggested changes Feb 25, 2022

View reviewed changes

udpated try/except

bde9af2

navinsoni approved these changes Feb 25, 2022

View reviewed changes

Merge branch 'aws:dev' into jeniyat/data-serializer

fe0bf46

jeniyat merged commit 5c64e6c into aws:dev Feb 28, 2022

jeniyat deleted the jeniyat/data-serializer branch February 28, 2022 16:35

jeniyat added the type: feature request label Feb 28, 2022

philschmid reviewed Mar 1, 2022

View reviewed changes

shreyapandit pushed a commit that referenced this pull request Mar 1, 2022

feature: Data Serializer (#2956)

086258d

shreyapandit pushed a commit that referenced this pull request Mar 4, 2022

feature: Data Serializer (#2956)

d292ee1

jerrypeng7773 pushed a commit to jerrypeng7773/sagemaker-python-sdk that referenced this pull request Apr 19, 2022

feature: Data Serializer (aws#2956)

e9c73fa

Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026

feature: Data Serializer (aws#2956)

c9f8bd0

Uh oh!

Conversation

jeniyat commented Feb 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Checklist

General

Tests

Uh oh!

sagemaker-bot commented Feb 23, 2022

AWS CodeBuild CI Report

Uh oh!

codecov-commenter commented Feb 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sagemaker-bot commented Feb 23, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 23, 2022

AWS CodeBuild CI Report

Uh oh!

navinsoni Feb 23, 2022

Choose a reason for hiding this comment

Uh oh!

navinsoni Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

jeniyat Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

navinsoni left a comment

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

mufaddal-rohawala Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

jeniyat Feb 24, 2022

Choose a reason for hiding this comment

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

Uh oh!

sagemaker-bot commented Feb 24, 2022

AWS CodeBuild CI Report

jeniyat commented Feb 23, 2022 •

edited

Loading

codecov-commenter commented Feb 23, 2022 •

edited

Loading

philschmid Mar 1, 2022 •

edited

Loading