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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not validate for unevaluatedProperties #1075

Conversation

ikonst
Copy link
Contributor

@ikonst ikonst commented Mar 28, 2023

It appears that without checking is_valid the test suite still passes. The invalid properties already produce an error, so that the "unevaluated property" error is never needed to achieve a validation failure. On the contrary, when all property validation succeeds, "unevaluated property" could still produce an error.

Arguably, producing an error for expected but invalid properties is also confusing, being a "double jeopardy" of validation failures.


馃摎 Documentation preview 馃摎: https://python-jsonschema--1075.org.readthedocs.build/en/1075/

@ikonst ikonst marked this pull request as draft March 28, 2023 01:30
@ikonst ikonst marked this pull request as ready for review March 28, 2023 01:54
@Julian
Copy link
Member

Julian commented Mar 28, 2023

Nice! Indeed looks like this is a decent (~40%) speedup for the (unevaluatedProperties-specific) microbenchmark that I've been referring to for the new release.

Obviously I can believe there's gaps in the precise output for unevaluatedProperties (especially given the spec ambiguities), so this is potentially in need of revisiting later when unevaluatedProperties gets another look, but certainly seems like a win for now. Well done, merging.

@Julian Julian merged commit 27d3608 into python-jsonschema:main Mar 28, 2023
121 checks passed
@ikonst ikonst deleted the 2023-03-27-unevaluatedProperties-do-not-validate branch March 28, 2023 21:24
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.

None yet

2 participants