Skip to content

Pull down the include_default_values argument to to_json #405

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 3 commits into from
Aug 8, 2022
Merged

Pull down the include_default_values argument to to_json #405

merged 3 commits into from
Aug 8, 2022

Conversation

SamuelYvon
Copy link
Contributor

The to_json method does not include the include_default_values
option from to_dict. While the implementation to to_json is
almost litteraly only a call to to_dict() (and thus trivial to
replace), it is more convenient for users to not have to reimplement it
when the option is desired.

The `to_json` method does not include the `include_default_values`
option from `to_dict`. While the implementation to `to_json` is
almost litteraly only a call to `to_dict()` (and thus trivial to
replace), it is more convenient for users to not have to reimplement it
when the option is desired.
@Gobot1234
Copy link
Collaborator

Might also be a good idea to add support for the casing arg, not too fussed about it though. Thanks for the PR.

@SamuelYvon
Copy link
Contributor Author

SamuelYvon commented Aug 3, 2022

Added.

BTW the latest version on pypi is quite old (and buggy). It would be worthwhile updating.

@Gobot1234
Copy link
Collaborator

Would you mind also adding a couple of tests for this (1 or 2 should be enough). The newest version on pypi was released yesterday (I hope it's not too buggy) you can install it now but you need to use the --pre flag

@SamuelYvon
Copy link
Contributor Author

(I hope it's not too buggy)

I was using 1.2.5 I believe, which was not working properly w.r.t the oneof. I installed the pre-release and it works fine.

@SamuelYvon
Copy link
Contributor Author

I'll add tests when I get a moment today

@SamuelYvon
Copy link
Contributor Author

@Gobot1234 You got the tests :)

@Gobot1234
Copy link
Collaborator

Thanks!

@Gobot1234 Gobot1234 merged commit 591ec5e into danielgtaylor:master Aug 8, 2022
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