From f1b5a8f85ed1077f809f0c6cc909104c739bd14a Mon Sep 17 00:00:00 2001 From: Brett Date: Tue, 10 Dec 2024 09:13:52 -0500 Subject: [PATCH] dial back severity of ambiguity --- asdf/_asdf.py | 4 +++ asdf/_node_info.py | 33 +++++------------ asdf/_tests/test_info.py | 77 ++++++++++++---------------------------- asdf/exceptions.py | 6 ---- changes/1875.bugfix.rst | 2 +- 5 files changed, 37 insertions(+), 85 deletions(-) diff --git a/asdf/_asdf.py b/asdf/_asdf.py index 434733f87..b63349dbd 100644 --- a/asdf/_asdf.py +++ b/asdf/_asdf.py @@ -1378,6 +1378,10 @@ def schema_info(self, key="description", path=None, preserve_list=True, refresh_ """ Get a nested dictionary of the schema information for a given key, relative to the path. + This method will only return unambiguous info. If a property is subject to multiple + subschemas or contains ambiguous entries (multiple titles) no result will be returned + for that property. + Parameters ---------- key : str diff --git a/asdf/_node_info.py b/asdf/_node_info.py index 0a88ddaa2..048e3ad1b 100644 --- a/asdf/_node_info.py +++ b/asdf/_node_info.py @@ -1,7 +1,6 @@ import re from collections import namedtuple -from .exceptions import AsdfInfoResolutionError from .schema import load_schema from .treeutil import get_children @@ -50,8 +49,7 @@ def _get_subschema_for_property(schema, key): if subschema is not None: # We can't resolve a valid subschema under a "not" since # we'd have to know how to invert a schema - msg = f"schema info could not be determined for {key} since " f"it is nested under a 'not'." - raise AsdfInfoResolutionError(msg) + return None for combiner in ("allOf", "oneOf", "anyOf"): for combined_schema in schema.get(combiner, []): @@ -59,17 +57,10 @@ def _get_subschema_for_property(schema, key): if subschema is not None: applicable.append(subschema) - if not applicable: - return None - - if len(applicable) > 1: - msg = ( - f"schema info could not be determined for {key} since " - f"{len(applicable)} possibly applicable schemas were found." - ) - raise AsdfInfoResolutionError(msg) - - return applicable[0] + # only return the subschema if we found exactly 1 applicable + if len(applicable) == 1: + return applicable[0] + return None def _get_schema_key(schema, key): @@ -83,17 +74,11 @@ def _get_schema_key(schema, key): possible = _get_schema_key(combined_schema, key) if possible is not None: applicable.append(possible) - if not applicable: - return None - if len(applicable) > 1: - msg = ( - f"schema info could not be determined for {key} since " - f"{len(applicable)} possibly applicable schemas were found." - ) - raise AsdfInfoResolutionError(msg) - - return applicable[0] + # only return the property if we found exactly 1 applicable + if len(applicable) == 1: + return applicable[0] + return None def create_tree(key, node, identifier="root", filters=None, refresh_extension_manager=False): diff --git a/asdf/_tests/test_info.py b/asdf/_tests/test_info.py index 7b28961ad..cb8917c46 100644 --- a/asdf/_tests/test_info.py +++ b/asdf/_tests/test_info.py @@ -7,7 +7,6 @@ import pytest import asdf -from asdf.exceptions import AsdfInfoResolutionError from asdf.extension import ExtensionManager, ExtensionProxy, ManifestExtension from asdf.resource import DirectoryResourceMapping @@ -734,48 +733,29 @@ def test_node_property(schema, expected): @pytest.mark.parametrize( - "schema, msg", + "schema", [ - ({"not": {"properties": {"foo": {"type": "object"}}}}, "nested under a 'not'"), - ( - {"properties": {"foo": {"type": "object"}}, "allOf": [{"properties": {"foo": {"type": "object"}}}]}, - "2 possibly applicable schemas", - ), - ( - {"properties": {"foo": {"type": "object"}}, "anyOf": [{"properties": {"foo": {"type": "object"}}}]}, - "2 possibly applicable schemas", - ), - ( - {"properties": {"foo": {"type": "object"}}, "oneOf": [{"properties": {"foo": {"type": "object"}}}]}, - "2 possibly applicable schemas", - ), - ( - { - "allOf": [{"properties": {"foo": {"type": "object"}}}], - "anyOf": [{"properties": {"foo": {"type": "object"}}}], - }, - "2 possibly applicable schemas", - ), - ( - { - "anyOf": [{"properties": {"foo": {"type": "object"}}}], - "oneOf": [{"properties": {"foo": {"type": "object"}}}], - }, - "2 possibly applicable schemas", - ), - ( - { - "oneOf": [{"properties": {"foo": {"type": "object"}}}], - "allOf": [{"properties": {"foo": {"type": "object"}}}], - }, - "2 possibly applicable schemas", - ), + {"not": {"properties": {"foo": {"type": "object"}}}}, + {"properties": {"foo": {"type": "object"}}, "allOf": [{"properties": {"foo": {"type": "object"}}}]}, + {"properties": {"foo": {"type": "object"}}, "anyOf": [{"properties": {"foo": {"type": "object"}}}]}, + {"properties": {"foo": {"type": "object"}}, "oneOf": [{"properties": {"foo": {"type": "object"}}}]}, + { + "allOf": [{"properties": {"foo": {"type": "object"}}}], + "anyOf": [{"properties": {"foo": {"type": "object"}}}], + }, + { + "anyOf": [{"properties": {"foo": {"type": "object"}}}], + "oneOf": [{"properties": {"foo": {"type": "object"}}}], + }, + { + "oneOf": [{"properties": {"foo": {"type": "object"}}}], + "allOf": [{"properties": {"foo": {"type": "object"}}}], + }, ], ) -def test_node_property_error(schema, msg): +def test_node_property_error(schema): ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) - with pytest.raises(AsdfInfoResolutionError, match=msg): - ni.get_schema_for_property("foo") + assert ni.get_schema_for_property("foo") == {} @pytest.mark.parametrize( @@ -786,23 +766,12 @@ def test_node_property_error(schema, msg): ({"oneOf": [{"title": "foo"}]}, "foo"), ({"anyOf": [{"title": "foo"}]}, "foo"), ({"not": {"title": "foo"}}, None), + ({"allOf": [{"title": "foo"}, {"title": "bar"}]}, None), + ({"oneOf": [{"title": "foo"}, {"title": "bar"}]}, None), + ({"anyOf": [{"title": "foo"}, {"title": "bar"}]}, None), + ({"allOf": [{"title": "foo"}, {"title": "bar"}]}, None), ], ) def test_node_info(schema, expected): ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) assert ni.info == expected - - -@pytest.mark.parametrize( - "schema", - [ - {"allOf": [{"title": "foo"}, {"title": "bar"}]}, - {"oneOf": [{"title": "foo"}, {"title": "bar"}]}, - {"anyOf": [{"title": "foo"}, {"title": "bar"}]}, - {"allOf": [{"title": "foo"}, {"title": "bar"}]}, - ], -) -def test_node_info_failure(schema): - ni = asdf._node_info.NodeSchemaInfo.from_root_node("title", "root", {}, schema) - with pytest.raises(AsdfInfoResolutionError, match="2 possibly applicable schemas"): - ni.info diff --git a/asdf/exceptions.py b/asdf/exceptions.py index cfdef90b5..0851a8baa 100644 --- a/asdf/exceptions.py +++ b/asdf/exceptions.py @@ -84,9 +84,3 @@ class AsdfSerializationError(RepresenterError): that the object does not have a supporting asdf Converter and needs to be manually converted to a supported type. """ - - -class AsdfInfoResolutionError(ValueError): - """ - An attempt to lookup schema for an attribute failed. - """ diff --git a/changes/1875.bugfix.rst b/changes/1875.bugfix.rst index fcdc9d162..89960753a 100644 --- a/changes/1875.bugfix.rst +++ b/changes/1875.bugfix.rst @@ -1 +1 @@ -Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc). Raise AsdfInfoResolutionError for ambiguous info. +Improve ``schema_info`` handling of schemas with combiners (allOf, anyOf, etc).