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

set/tuple/datatime data types not JSON serializable #387

Open
richard-to opened this issue Jun 8, 2024 · 3 comments
Open

set/tuple/datatime data types not JSON serializable #387

richard-to opened this issue Jun 8, 2024 · 3 comments
Labels
bug Something isn't working good first issues Good for first-time contributors

Comments

@richard-to
Copy link
Collaborator

  • The tuple is serializable but gets deserialized back to a list
  • Set is not serializable at all
  • Datetime also not serializable:

I think we can add support for these by doing some custom encoding parameters similar to what we did for Pandas in MesopJSONEncoder

For set and tuple, probably can do something like:

      if isinstance(obj, set):
        return {"[set]": obj}

Then in decode_mesop_json_state_hook

    if "[set]" in dct:
      return set(dct["[set]"]),
``

Haven't tested it though. Depends on how the encode decoding works. If it works it's way upwards, then this should work. But if the serialization/deserialization works from root to leaf, then wouldn't work for nested types. But I imagine it should work from leaf to root

--------

Datetime would be similar I think. But need to determine the format we want to store the date as. Some ISO standard or unix timetamp. Maybe some kind of ISO standard especially if we need to handle time zones.

----

Code link: https://github.com/google/mesop/blob/main/mesop/dataclass_utils/dataclass_utils.py
@richard-to richard-to added bug Something isn't working good first issues Good for first-time contributors labels Jun 8, 2024
@richard-to
Copy link
Collaborator Author

One other note is that for set/tuple changes we may need to adjust the paths for the state diff output to apply the diffs correctly.

@zacharias1219
Copy link
Contributor

Hey can I work on this, this seems to be similar to the one I did before, would be easier.

@richard-to
Copy link
Collaborator Author

Someone was working on this. They did the datetime part. I don't know if they plan to come back to work on the set / tuple, so you may want to wait a week before attempting this.

Also the implementation for tuple may not be straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issues Good for first-time contributors
Projects
None yet
Development

No branches or pull requests

2 participants