Skip to content
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.

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

[Feature Request] Upload Artifact Fails on Non-Standard Dictionaries #452

Closed
jalexand3r opened this issue Sep 17, 2021 · 4 comments
Closed
Labels
Feature Request Feature Request - Support w/ :+1: reaction

Comments

@jalexand3r
Copy link
Contributor

jalexand3r commented Sep 17, 2021

Currently upload_artifact only checks whether an object is a dict before dumping to JSON:

elif isinstance(artifact_object, dict):

However, there is no guarantee that values within the dictionary are JSON serializable (e.g. dictionaries of DataFrames will fail).

Although it may not be best practice to upload dictionaries of objects instead of uploading each object individually, there are good use cases for the former. It would be nice to have error handling that defaults to pickling objects if auto_pickle is true and standard data type uploading fails.

@bmartinn
Copy link
Member

Hi @jalexand3r

Although it may not be best practice to upload dictionaries of objects instead of uploading each object individually, there are good use cases for the former. It would be nice to have error handling that defaults to pickling objects if auto_pickle is true and standard data type uploading fails.

This is a good idea. So let's assume we try to upload a non-jsonable dict, we output a warning (e.g JSON serialization failed, falling back to pickle), then we would just pickle the dict itself, and upload the pkl file as artifact.
Is this what you had in mind?

@jalexand3r
Copy link
Contributor Author

I would check the auto_pickle argument after the warning and before pickling, but yes, that is essentially what I had in mind.

@bmartinn
Copy link
Member

Sounds good to be, @jalexand3r do you mind renaming the issue to feature request ?

@bmartinn bmartinn added the Feature Request Feature Request - Support w/ :+1: reaction label Sep 20, 2021
@jalexand3r jalexand3r changed the title Upload Artifact Fails on Non-Standard Dictionaries [Feature Request] Upload Artifact Fails on Non-Standard Dictionaries Sep 20, 2021
@jkhenning
Copy link
Member

Closing this as it was already released. Please reopen if required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Feature Request - Support w/ :+1: reaction
Projects
None yet
Development

No branches or pull requests

3 participants