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

feat: expose a visitor to deserialize a pyobject #65

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

Conversation

Stargateur
Copy link
Contributor

@Stargateur Stargateur commented Jun 16, 2024

Working with pyo3 I end up needed this, user can implement it themself but I though it would be nice to have it in pythonize. Tell me what you think.

If the feature is not desired to avoid a dependency serde-transcode I think only expose the visitor is already good enough.

@Stargateur Stargateur marked this pull request as ready for review June 16, 2024 14:13
Copy link
Owner

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Sorry for the long delayed reply here. So I'm trying to understand the serialize / deserialize implementations. It looks like the idea (by virtue of using serde_transcode) is that this will allow serialization of a PyObject by first pulling it through depythonize and then pushing that through some other serializer (to e.g. produce a JSON string). Similar for the reverse, to go straight from a JSON string to a PyObject without first needing to go through some intermediate in-memory form.

Is that how you would describe it?

where
D: serde::Deserializer<'de>,
{
Python::with_gil(|py| deserializer.deserialize_any(crate::PyObjectVisitor::new(py)))
Copy link
Owner

Choose a reason for hiding this comment

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

Should this also use serde_transcode? I think in that way the PyObjectVisitor might not be needed, and instead we use Pythonizer here in the same way we use Depythonizer above?

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