Skip to content

Fix/freeze dict keys #3

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

Conversation

juliusfrost
Copy link
Collaborator

No description provided.

@juliusfrost juliusfrost force-pushed the fix/freeze_dict_keys branch from 91a4128 to d873e56 Compare April 26, 2023 02:50
@nielstron
Copy link

Thanks for the suggestion. I am actually not sure if we want to freeze everything, even if not needed. But it probably doesn't hurt either 🤔

@cffls maybe you know about use cases of unfrozen serialized output?

@cffls
Copy link
Collaborator

cffls commented Apr 26, 2023

As long as we don't change the original object, I think it is okay to freeze everything before serializing them. I can't think of cases where users would want to modify the output of to_primitives. All modification should be done through the original objects.

@nielstron nielstron merged commit 07c7e3e into OpShin:fix/freeze_dict_keys Apr 26, 2023
@juliusfrost juliusfrost deleted the fix/freeze_dict_keys branch April 26, 2023 17:41
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.

3 participants