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

Enable pickling #942

Merged
merged 3 commits into from
Nov 29, 2021
Merged

Enable pickling #942

merged 3 commits into from
Nov 29, 2021

Conversation

varunagrawal
Copy link
Collaborator

@varunagrawal varunagrawal commented Nov 29, 2021

Enable pickling so we can deep copy a bunch of things in the Python wrapper.

The wrapper expects declaration of the pickle method in the interface file and will use the underlying serialize method of the class. The wrapper has had this feature for a while, but the classes updated in this PR didn't have the pickling enabled.

@varunagrawal varunagrawal added the high-priority Need this done quickly label Nov 29, 2021
Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

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

How does this PR enable pickling? Does this rely on functionality in wrap? Is that the wrap version currently included with GTSAM? Add more context in PR comment and req-request review?

@varunagrawal
Copy link
Collaborator Author

@dellaert added details on pickling with the wrapper.

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

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

Not holding up this PR, but why do we have a separate pickle and serialize method? Is not every serializable class pickalable?

@varunagrawal
Copy link
Collaborator Author

It is. I noticed this exact problem when making this PR, but this requires a change to the wrapper. I can do it over the next week after higher priority items have been completed. 🙂

@varunagrawal varunagrawal merged commit 3637f0d into wrap/isam2 Nov 29, 2021
@varunagrawal varunagrawal deleted the feature/pickling branch November 29, 2021 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority Need this done quickly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants