Skip to content

Initial serde support #241

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 1 commit into from
Oct 22, 2020
Merged

Initial serde support #241

merged 1 commit into from
Oct 22, 2020

Conversation

quark-zju
Copy link
Contributor

Converts between serde types and PyObject. This can make FromPyObject/
ToPyObject implementations easier.

Copy link
Collaborator

@markbt markbt left a comment

Choose a reason for hiding this comment

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

This looks very cool.

I'll leave it up for others to comment on, but right now I can't see any issues with it, aside from maybe making it opt-in by default.

Thanks!

Cargo.toml Outdated
@@ -49,7 +54,10 @@ path = "python3-sys"
version = "0.5.1"

[features]
default = ["python3-sys"]
default = ["python3-sys", "serde-support"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to make this a default? I would assume existing users don't care about it, and we wouldn't want them to have to pull in an extra stack of dependencies if they're not using serde.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I only wanted CI to run the tests. I updated Makefile for that and made the feature default off.

Converts between serde types and PyObject. This can make FromPyObject/
ToPyObject implementations easier.

The feature is gated by `serde-convert`.
@markbt markbt merged commit 68ace18 into dgrunwald:master Oct 22, 2020
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