Skip to content

WIP: Add custom json encoder and decoder - #533

Closed
constantinpape wants to merge 1 commit into
zarr-developers:mainfrom
constantinpape:master
Closed

WIP: Add custom json encoder and decoder#533
constantinpape wants to merge 1 commit into
zarr-developers:mainfrom
constantinpape:master

Conversation

@constantinpape

@constantinpape constantinpape commented Jan 23, 2020

Copy link
Copy Markdown

Add custom JsonEncoder / JsonDecoder to properly handle nan and inf attributes, see #412.
Also adds a test to ensure these values are properly encoded / decoded.
This is still WIP because:

  • I haven't tested the solution myself yet.
  • Potentially one could simplify the code here and here with this solution, but I am not sure this is a good idea, because I don't know if we can always assume that the metadata passes through zarr.util.json_dumps/json_loads
  • Potentially one could also add json encoding for numpy dtypes; this is an error that always annoys me a lot, but it's unrelated to Special treatment of NaN in JSON user attributes #412.

TODO:

  • Add unit tests and/or doctests in docstrings
  • Add docstrings and API docs for any new/modified user-facing classes and functions
  • New/modified features documented in docs/tutorial.rst
  • Changes documented in docs/release.rst
  • AppVeyor and Travis CI passes
  • Test coverage is 100% (Coveralls passes)

@jakirkham

Copy link
Copy Markdown
Member

Should the encoder/decoder bits be moved to Numcodecs?

@constantinpape

Copy link
Copy Markdown
Author

Should the encoder/decoder bits be moved to Numcodecs?

Happy to move it there if you think that would make more sense. Would you put it here?

@constantinpape

Copy link
Copy Markdown
Author

Tests pass, so the solution seems to be viable.

@joshmoore

Copy link
Copy Markdown
Member

Sorry, @constantinpape. Somehow I missed this when reviewing #933 which maybe should been considered together.

@joshmoore

Copy link
Copy Markdown
Member

Zarr/JSON encoding came up again yesterday during the community call. I will try to resolve the conflicts here and see how things stand.

@jhamman jhamman added the stale label Dec 7, 2023
@jhamman

jhamman commented Dec 7, 2023

Copy link
Copy Markdown
Member

This has unfortunately gone stale. Will it be revived or should we close this?

@jhamman

jhamman commented Feb 13, 2024

Copy link
Copy Markdown
Member

Closing this out as stale. We'll keep the feature request open.

@jhamman jhamman closed this Feb 13, 2024
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.

4 participants