Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

JSON serializer: predictor.predict accepts dictionaries#62

Merged
lukmis merged 3 commits intoaws:masteraws/sagemaker-python-sdk:masterfrom
andremoeller:predict-dictionariesandremoeller/sagemaker-python-sdk:predict-dictionariesCopy head branch name to clipboard
Feb 15, 2018
Merged

JSON serializer: predictor.predict accepts dictionaries#62
lukmis merged 3 commits intoaws:masteraws/sagemaker-python-sdk:masterfrom
andremoeller:predict-dictionariesandremoeller/sagemaker-python-sdk:predict-dictionariesCopy head branch name to clipboard

Conversation

@andremoeller
Copy link
Contributor

No description provided.

@andremoeller andremoeller requested a review from mvsusp January 30, 2018 00:08

if isinstance(data, dict):
if not len(data.keys()) > 0:
raise ValueError("empty dictionary can't be serialized")
Copy link
Contributor

Choose a reason for hiding this comment

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

Any special reason for handling the empty dictionary case? An empty dictionary is a valid json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Valid JSON: sure, but similar to an empty list, what would I attempt to be predicting without data? I cannot think of such a use case where I'd expect my payload to be empty. My prediction response wouldn't depend on my payload. But maybe it's better to be permissive here and with the list. Let me know what you think, I can change this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I noticed that you are applying the same previously applied pattern to check if it is empty or not. I'm ok with that.


if isinstance(data, dict):
if not len(data.keys()) > 0:
raise ValueError("empty dictionary can't be serialized")
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I noticed that you are applying the same previously applied pattern to check if it is empty or not. I'm ok with that.

@lukmis lukmis merged commit d47f6d1 into aws:master Feb 15, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)

When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.

instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.

* Add wrapper for LDA. (aws#56)

Update CHANGELOG and bump the version number.

* Add support for async fit() (aws#59)

when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.

_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()

* Fix Estimator role expansion (aws#68)

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

* Add FM and LDA to the documentation. (aws#66)

* Fix description of an argument of sagemaker.session.train (aws#69)

* Fix description of an argument of sagemaker.session.train

'input_config' should be an array which has channel objects.

* Add a link to the botocore docs

* Use 'list' instead of 'array' in the description

* Add ntm algorithm with doc, unit tests, integ tests (aws#73)

* JSON serializer: predictor.predict accepts dictionaries (aws#62)

Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test

* Fixing timeouts for PCA async integration test. (aws#78)

Execute tf_cifar test without logs to eliminate delay to detect that job has finished.

* Fixes in LinearLearner and unit tests addition. (aws#77)

* Print out billable seconds after training completes (aws#30)

* Added: print out billable seconds after training completes

* Fixed: test_session.py to pass unit tests

* Fixed: removed offending tzlocal()

* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)

* Support TensorFlow-1.5.0 and MXNet-1.0.0  (aws#82)

* Update .gitignore to ignore pytest_cache.

* Support TensorFlow-1.5.0 and MXNet-1.0.0

* Update and refactor tests. Add tests for fw_utils.

* Fix typo.

* Update changelog for 1.1.0 (aws#85)
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Scikit bring your own

Merging this in preparation for Monday's meeting.
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.