Use context managers instead of unrolled scopes #429
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't default to urlopen when unknown scheme encountered (`raise ValueError(scheme)``)
Facilitate this by modifying `in_scope` to only push scopes which boolean evaluate to True. Update `LegacyRefResolver` in test suite to support this (no-op `in_scope` context manager).
|
See my comments on the other PR which would likely apply here too, it's certainly harder to deal with this if it's dependent on the other one, but the one change I can see here does break backwards compatibility as well, so depending on what the goal is here it can't be merged as-is, but possibly there's a solution that covers whatever the goal is. |
|
Leave this for now. |
Julian
added a commit
that referenced
this pull request
Jun 25, 2022
f0f619d19 Merge pull request #560 from json-schema-org/ether/fix-383 fa791ada9 remove tests that are not valid for draft7 0f341dca0 Merge pull request #429 from notEthan/decimal_minItems 32ec6305c test validation of keywords which expect integers when their value has a decimal 547330dd6 Tweak the markdown table syntax in the README. 28ed3022c Restructure the README further. 45d6e5e7f README: Add new content and update the structure 4de0966a7 Merge pull request #558 from yakimun/replace-definitions-with-defs aad0f350a Replace definitions with for 2019 and later drafts 0b777ffa2 Merge branch 'ether/more-anchor-and-id-tests' d593591d7 Forward port the just-added tests to 2020 and next. e157bc0b7 squash: use a unique $id to prevent namespace collisions across tests 1d5583de6 some tests of the interactions between $id, $anchor and $ref 07c45c0d7 Restore the dependencies tests as optional for 2019, 2020, and next git-subtree-dir: json git-subtree-split: f0f619d19696a81f7683116f707a51c34150d28f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends upon GH-428.
I've seen #158 which suggests that this is a deliberate feature. Perhaps then this isn't appropriate.
However, in https://github.com./agoose77/jsonschema/blob/master/jsonschema/_validators.py#L304 we have more complicated logic where we look for the contextmanager, and use it if available, otherwise defaulting to an unrolled loop. As far as I can tell, this is to facilitate the mock ref resolver which only provides the context manager. If performance is still a concern, I suggest we eliminate one branch.