Skip to content

Self-Contained JSON file format #412

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

Merged
merged 14 commits into from
Nov 15, 2023
Merged

Self-Contained JSON file format #412

merged 14 commits into from
Nov 15, 2023

Conversation

InnocentBug
Copy link

Description

The SDK can produce fully self-contained JSON string for node graphs.
Such a node graph does not contain any UUID reference (self-contained) and can be super helpful for users if they want to store their work in a JSON file before uploading it to CRIPT.

@bearmit expressed that he would like this feature.

Changes

Also added a hint about using kwargs to beautify JSON strings into the documentation.

Tests

A test is included to ensures that self-containing JSON can be parsed by cript.load_nodes_from_json.

Known Issues

Notes

Checklist

  • My name is on the list of contributors (CONTRIBUTORS.md) in the pull request source branch.
  • I have updated the documentation to reflect my changes.

@InnocentBug InnocentBug requested a review from nh916 November 13, 2023 23:48
@InnocentBug InnocentBug self-assigned this Nov 13, 2023
Copy link

trunk-io bot commented Nov 13, 2023

Merging to develop in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

changed from `get_self_contained_json` to `get_expanded_json`
* added comments
* renamed variables to be more self documenting and obvious
* added type hinting where it would help
* renamed test to be more self documenting and make more sense
@nh916 nh916 mentioned this pull request Nov 14, 2023
2 tasks
* changed name from `mybigsmiles` that triggered spelling error CI to something that would not trigger it and is more explicit and better anyways.
  * replaced it with `my material 1 bigsmiles` and `my material 2 bigsmiles`
nh916
nh916 previously approved these changes Nov 15, 2023
1. A complex project node can be serialized into an expanded JSON string, without UUID placeholders.
2. The expanded JSON can be deserialized into a node that is equivalent to the original node.
"""
expanded_project_json: str = complex_project_node.get_expanded_json()
Copy link
Contributor

Choose a reason for hiding this comment

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

in hindsight, I probably should have named this project_expanded_json to make more sense, but I think it'll pass for now

@nh916 nh916 mentioned this pull request Nov 15, 2023
@nh916 nh916 self-requested a review November 15, 2023 20:18
# assert the expanded JSON was correctly deserialized to project node
assert deserialized_project_node == complex_project_node

short_json: str = complex_project_node.json
Copy link
Contributor

Choose a reason for hiding this comment

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

this should have also been condensed_json instead to make it more self documenting and clearer

Copy link
Author

Choose a reason for hiding this comment

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

agreed

>>> # ============= Get long form JSON =============
>>> long_form_json = my_project.get_expanded_json(indent=4)

???+ info "Short JSON VS Long JSON"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have called this section Condensed JSON VS Expanded JSON, but it is okay for now and we can come back to it later

Copy link
Author

Choose a reason for hiding this comment

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

agreed

@InnocentBug InnocentBug merged commit 8cc2ae0 into develop Nov 15, 2023
@InnocentBug InnocentBug deleted the json-file-format branch November 15, 2023 20:48
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.

2 participants