-
Notifications
You must be signed in to change notification settings - Fork 0
Work towards compatibility for 4.17 and 4.18 #4
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| """ | ||
| import pkgutil | ||
| from collections.abc import Mapping | ||
| import importlib_metadata | ||
|
|
||
| from asdf_standard import DirectoryResourceMapping as _DirectoryResourceMapping | ||
|
|
||
|
|
@@ -168,9 +169,16 @@ def __repr__(self): | |
| return f"<ResourceManager len: {self.__len__()}>" | ||
|
|
||
|
|
||
| _JSONSCHEMA_URI_TO_FILENAME = { | ||
| "http://json-schema.org/draft-04/schema": "draft4.json", | ||
| } | ||
| if importlib_metadata.version('jsonschema') >= '4.18': | ||
| USE_JSONSCHEMA_SPECIFICATIONS = True | ||
| _JSONSCHEMA_URI_TO_FILENAME = { | ||
| "http://json-schema.org/draft-04/schema": "metaschema.json", | ||
| } | ||
| else: | ||
| USE_JSONSCHEMA_SPECIFICATIONS = False | ||
| _JSONSCHEMA_URI_TO_FILENAME = { | ||
| "http://json-schema.org/draft-04/schema": "draft4.json", | ||
| } | ||
|
|
||
|
|
||
| class JsonschemaResourceMapping(Mapping): | ||
|
|
@@ -181,7 +189,10 @@ class JsonschemaResourceMapping(Mapping): | |
|
|
||
| def __getitem__(self, uri): | ||
| filename = _JSONSCHEMA_URI_TO_FILENAME[uri] | ||
| return pkgutil.get_data("jsonschema", f"schemas/{filename}") | ||
| if USE_JSONSCHEMA_SPECIFICATIONS: | ||
| return pkgutil.get_data("jsonschema_specifications", f"schemas/draft4/{filename}") | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahhhh so that's where that got to! I was super confused when I didn't see it in either referencing or jsonschema. |
||
| else: | ||
| return pkgutil.get_data("jsonschema", f"schemas/{filename}") | ||
|
|
||
| def __len__(self): | ||
| return len(_JSONSCHEMA_URI_TO_FILENAME) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,16 @@ | |
|
|
||
| import numpy as np | ||
| import yaml | ||
| import importlib_metadata | ||
| if importlib_metadata.version('jsonschema') >= "4.18": | ||
| USE_REFERENCING = True | ||
| import referencing | ||
| from referencing.exceptions import Unretrievable as RefError | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe there are actually two exceptions we need to catch here, |
||
| else: | ||
| from jsonschema.exceptions import RefResolutionError as RefError | ||
| USE_REFERENCING = False | ||
| from jsonschema import validators as mvalidators | ||
| from jsonschema.exceptions import RefResolutionError, ValidationError | ||
| from jsonschema.exceptions import ValidationError | ||
|
|
||
| from . import constants, extension, generic_io, reference, tagged, treeutil, util, versioning, yamlutil | ||
| from .config import get_config | ||
|
|
@@ -215,7 +223,10 @@ def has_seen_pair(self, schema, instance): | |
| return (id(schema), id(instance)) in self._seen_id_pairs | ||
|
|
||
| def create(self, schema, resolver): | ||
| return self._validator_class(schema, resolver=resolver) | ||
| if USE_REFERENCING: | ||
| return self._validator_class(schema, registry=resolver) | ||
| else: | ||
| return self._validator_class(schema, resolver=resolver) | ||
|
|
||
|
|
||
| class _CycleCheckingValidatorProxy: | ||
|
|
@@ -384,13 +395,20 @@ def get_schema(url): | |
| # remote schemas here in `load_schema`, so we don't need | ||
| # jsonschema to do it on our behalf. Setting it to `True` | ||
| # counterintuitively makes things slower. | ||
| return mvalidators.RefResolver( | ||
| "", | ||
| {}, | ||
| cache_remote=False, | ||
| handlers=handlers, | ||
| urljoin_cache=urljoin_cache, | ||
| ) | ||
| if USE_REFERENCING: | ||
| def retrieve_schema(url): | ||
| schema = get_schema(url) | ||
| resource = referencing.Resource(schema, specification=referencing.jsonschema.DRAFT4) | ||
| return resource | ||
| return referencing.Registry({}, retrieve=retrieve_schema) | ||
| else: | ||
| return mvalidators.RefResolver( | ||
| "", | ||
| {}, | ||
| cache_remote=False, | ||
| handlers=handlers, | ||
| urljoin_cache=urljoin_cache, | ||
| ) | ||
|
|
||
|
|
||
| @lru_cache | ||
|
|
@@ -569,11 +587,18 @@ def _iter_errors(self, instance, _schema=None): | |
|
|
||
| for schema_uri in schema_uris: | ||
| try: | ||
| tag_schema = self._resolver.resolve(schema_uri)[1] | ||
| yield from self._json_schema_validator_factory.create(tag_schema, self._resolver).iter_errors( | ||
| node, | ||
| ) | ||
| except RefResolutionError: | ||
| if USE_REFERENCING: | ||
| tag_resource = self._resolver.get_or_retrieve(schema_uri) | ||
| resolver = tag_resource.value @ self._resolver | ||
| v = self._json_schema_validator_factory.create(tag_resource.value.contents, resolver) | ||
| v._resolver = resolver.resolver_with_root(tag_resource.value) | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was added when trying to debug why many tests were failing with errors like: which was tracked down to loss of the
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This I believe is a symptom of this issue: python-jsonschema/jsonschema#1061 Once we're able to specify the dialect we want, that specification object will be able to find the software schema's id field and use it as the base URI. |
||
| yield from v.iter_errors(node) | ||
| else: | ||
| tag_schema = self._resolver.resolve(schema_uri)[1] | ||
| yield from self._json_schema_validator_factory.create(tag_schema, self._resolver).iter_errors( | ||
| node, | ||
| ) | ||
| except RefError: | ||
| warnings.warn(f"Unable to locate schema file for '{tag}': '{schema_uri}'", AsdfWarning) | ||
|
|
||
| def validate(self, instance, _schema=None): | ||
|
|
@@ -803,21 +828,13 @@ def check_schema(schema, validate_default=True): | |
| validators = util.HashableDict(mvalidators.Draft4Validator.VALIDATORS.copy()) | ||
|
|
||
| if validate_default: | ||
| # The jsonschema library doesn't validate defaults | ||
| # on its own. | ||
| instance_validator = _create_json_schema_validator_factory().create(schema, _make_resolver(None)) | ||
| instance_scope = schema.get("id", "") | ||
|
|
||
| def _validate_default(validator, default, instance, schema): | ||
| if not validator.is_type(instance, "object"): | ||
| return | ||
|
|
||
| if "default" in instance: | ||
| instance_validator.resolver.push_scope(instance_scope) | ||
| try: | ||
| yield from instance_validator.descend(instance["default"], instance) | ||
| finally: | ||
| instance_validator.resolver.pop_scope() | ||
| get_validator(instance).validate(instance['default']) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well that certainly seems more better! |
||
| return | ||
|
|
||
| validators.update({"default": _validate_default}) | ||
|
|
||
|
|
@@ -841,5 +858,8 @@ def applicable_validators(schema): | |
| id_of=mvalidators.Draft4Validator.ID_OF, | ||
| applicable_validators=applicable_validators, | ||
| ) | ||
| validator = cls(meta_schema, resolver=resolver) | ||
| if USE_REFERENCING: | ||
| validator = cls(meta_schema, registry=resolver) | ||
| else: | ||
| validator = cls(meta_schema, resolver=resolver) | ||
| validator.validate(schema) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we should make this 4.18.0dev...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, another thing I didn't notice until after I merged is we're just comparing strings here. Need to use packaging.version.parse so that they get compared as version numbers.