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.

Sign up for GitHub

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

Jump to bottom

Fix serialization support for dataclasses #339

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Fix serialization support for dataclasses #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions recirq/serialization_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,9 @@ def wrap(cls):

cls._json_namespace_ = classmethod(lambda obj: namespace)

cls._json_dict_ = classmethod(lambda obj: cirq.obj_to_dict_helper(
obj, [f.name for f in dataclasses.fields(cls)]))
cls._json_dict_ = lambda obj: cirq.obj_to_dict_helper(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure, this is what I changed here:
#337
and it failed with the message "requiring positional argument obj"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am looking at https://quantumai.google/cirq/dev/serialization#adding_a_new_serializable_value and to my understanding _json_dict_ needs to be an instance method to return the instance data.

I left the _json_namespace_ a classmethod - as it was set in your PR. I have also updated the test_json_serializable_class unit test which fails at master, but passes in this PR. Finally, I did a quick test run of the failing notebooks data_collection.ipynb and data_analysis.ipynb which seem to work after this change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, yes, you are correct. I got json_dict and json_namespace confused.

obj, [f.name for f in dataclasses.fields(cls)]
)

if registry is not None:
if namespace is not None:
Expand Down
3 changes: 3 additions & 0 deletions recirq/serialization_utils_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,6 @@ def test_str_and_repr():

def test_json_serializable_class():
assert TestTask._json_namespace_() == "recirq.test_task"
test_task = TestTask(dataset_id="duck")
json_text = cirq.to_json(test_task)
assert test_task == recirq.read_json(json_text=json_text)
Loading