Skip to content

Add type & value fields to primitive json types #915

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dyackzan
Copy link

This aligns with how custom types are represented by the JsonExporter.

  • Update both toJson & fromJson functions in the JsonExporter
  • Add & update tests

@dyackzan dyackzan force-pushed the json-add-fields-to-primitive-types-base-btcpp branch 2 times, most recently from 7cff109 to c5561ac Compare January 29, 2025 23:01
This aligns with how custom types are represented by the JsonExporter.

* Update both toJson & fromJson functions in the JsonExporter
* Add & update tests

Co-authored by: David Sobek <david.sobek@picknik.ai>
@dyackzan dyackzan force-pushed the json-add-fields-to-primitive-types-base-btcpp branch from c5561ac to 4f6794d Compare January 29, 2025 23:06
@facontidavide
Copy link
Collaborator

  1. Is this change back-compatible with the previous JSON format?
  2. Can we opt-out if people don't want to use this format?

@facontidavide facontidavide self-requested a review January 31, 2025 15:58
@dyackzan
Copy link
Author

  1. If I understand your meaning correctly then I don't think this is backward compatible: if someone has a JSON serialization of their blackboard with generic types on it from before this change and they try to load it using exporter.fromJson with this change then it will fail and give them the message: "Missing fileld '__type'." I had to update the tests to account for this.
  2. I did not implement a way to opt out of this but I believe this way makes more sense for everything to have a __type field rather than just custom types

Are you thinking you don't want to release this in a non-breaking release if there is no way to opt out?

@facontidavide facontidavide self-assigned this Feb 5, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants