feature: all predictors support serializer/deserializer overrides #1997
Conversation
All framework *Predictor classes accept constructor arguments to override the default `serializer` & `deserializer` logic (like TensorFlowPredictor did already).
All Amazon algorithm *Predictor classes accept constructor arguments to override the default `serializer` & `deserializer` logic, in case users need to specify alternative inference formats.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thanks & sure thing - hopefully these fit the bill? |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Any way to re-start the CI checks without adding more commits? Seems like there was some sort of availability issue and they're stuck in "pending" |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available: #1958
Description of changes:
Give users ability to override the default serializer/deserializer behavior of
*Predictorclasses, without having to modify the non-API-documentedpredictor.serializerandpredictor.deserializerinstance variables - by explicitly exposing these options in all Predictor constructors.Previously, many predictor classes hard-coded their serializer/deserializer defaults in their constructor. Instead we can expose the same
serializeranddeserializerconstructor kwargs defined by the base class, and override their default values in the child.I elected to include Amazon algorithm predictors in this change as well (rather than just the framework predictors), because all the examples I found are capable of inferencing with multiple formats: So even though the default RecordIO-protobuf format may be most efficient, there might be reasons why a user would want to use one of the others (for example, maybe they want to use a format that's compatible with SageMaker Model Monitor?)
Testing done:
Unit tests run, some anecdotal testing with a couple of classes seems OK. No new tests introduced because no new code paths are introduced.
Merge Checklist
Put an
xin 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 passed the region in to all S3 and STS clients that I've initialized as part of this change.Tests
I have added tests that prove my fix is effective or that my feature works(if appropriate)I have used(if appropriate)unique_name_from_baseto create resource names in integ testsBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.