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

Support Hashable for json validation #10324

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Support Hashable for json validation #10324

merged 3 commits into from
Sep 6, 2024

Conversation

sydney-runkle
Copy link
Member

@sydney-runkle sydney-runkle commented Sep 5, 2024

Alternative to #10005 based on #10005 (comment)

Fix #9567

Thanks @kc0506 for the test inspo!

Copy link

cloudflare-workers-and-pages bot commented Sep 5, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 418ca00
Status: ✅  Deploy successful!
Preview URL: https://71485969.pydantic-docs.pages.dev
Branch Preview URL: https://hashable-json.pydantic-docs.pages.dev

View logs

Copy link

codspeed-hq bot commented Sep 5, 2024

CodSpeed Performance Report

Merging #10324 will not alter performance

Comparing hashable-json (418ca00) with main (852736c)

Summary

✅ 50 untouched benchmarks

Copy link
Contributor

github-actions bot commented Sep 5, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

Copy link
Contributor

@dmontagu dmontagu left a comment

Choose a reason for hiding this comment

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

I think you should add a test for the cases that fail validation from JSON (presumably nested JSON objects/arrays would?). I vaguely recall seeing a version where it would convert arrays to tuples and objects to frozendict; I think we should keep the behavior like this and not do that, but I think it's worth adding a test with a note indicating that it was added to document the current behavior.

Other than that, LGTM

@sydney-runkle sydney-runkle merged commit 0fdf06f into main Sep 6, 2024
62 checks passed
@sydney-runkle sydney-runkle deleted the hashable-json branch September 6, 2024 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

model_validate_json cannot validate Hashable
2 participants