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

Fix: Add wrapper to prep data for JSON encoding #22

Merged
merged 3 commits into from
Mar 21, 2023

Conversation

dosisod
Copy link
Contributor

@dosisod dosisod commented Mar 21, 2023

The pydantic_encode function doesn't work with dict. list, str, int, etc. objects out of the box, so I had to make a small wrapper to get it to work. I haven't tested this extensively, but I know it isn't throwing a validation error anymore.

Closes #21.

Copy link
Owner

@yanyongyu yanyongyu left a comment

Choose a reason for hiding this comment

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

according to the json standard lib, tuple is also a valid type for encoding.
https://github.com/python/cpython/blob/d149f15d637a323ac818a7fa719cad945828a04b/Lib/json/encoder.py#L414-L442

and, we may need to do a copy when converting?

githubkit/core.py Outdated Show resolved Hide resolved
githubkit/core.py Outdated Show resolved Hide resolved
githubkit/core.py Outdated Show resolved Hide resolved
@yanyongyu yanyongyu added the bug Something isn't working label Mar 21, 2023
Copy link
Owner

@yanyongyu yanyongyu left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@yanyongyu yanyongyu changed the title Add wrapper to prep data for JSON encoding Fix: Add wrapper to prep data for JSON encoding Mar 21, 2023
@yanyongyu yanyongyu merged commit 8d9a7b1 into yanyongyu:master Mar 21, 2023
@dosisod dosisod deleted the pydantic-encode branch March 21, 2023 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Object of type datetime is not JSON serializable
2 participants