From 4330bd56959c6c9e65f74d880fe4e27ba0941717 Mon Sep 17 00:00:00 2001 From: Brett Date: Sat, 16 Dec 2023 11:53:41 -0500 Subject: [PATCH 1/2] deprecate find_references calls during AsdfFile.__init__ and open --- asdf/_asdf.py | 17 ++++++--- asdf/_tests/test_deprecated.py | 17 +++++++++ asdf/_tests/test_reference.py | 66 ++++++++++++++++++++++++---------- asdf/reference.py | 6 +++- 4 files changed, 82 insertions(+), 24 deletions(-) diff --git a/asdf/_asdf.py b/asdf/_asdf.py index 88a8bcb6c..b09b4f8d1 100644 --- a/asdf/_asdf.py +++ b/asdf/_asdf.py @@ -169,6 +169,12 @@ def __init__( self._closed = False self._external_asdf_by_uri = {} self._blocks = BlockManager(uri=uri, lazy_load=lazy_load, memmap=not copy_arrays) + # this message is passed into find_references to only warn if + # a reference was found + find_ref_warning_msg = ( + "find_references during AsdfFile.__init__ is deprecated. " + "call AsdfFile.find_references after AsdfFile.__init__" + ) if tree is None: # Bypassing the tree property here, to avoid validating # an empty tree. @@ -183,10 +189,10 @@ def __init__( # Set directly to self._tree (bypassing property), since # we can assume the other AsdfFile is already valid. self._tree = tree.tree - self.find_references() + self.find_references(_warning_msg=find_ref_warning_msg) else: self.tree = tree - self.find_references() + self.find_references(_warning_msg=find_ref_warning_msg) self._comments = [] @@ -839,7 +845,8 @@ def _open_asdf( # to select the correct tag for us. tree = yamlutil.custom_tree_to_tagged_tree(AsdfObject(), self) - tree = reference.find_references(tree, self) + find_ref_warning_msg = "find_references during open is deprecated. call AsdfFile.find_references after open" + tree = reference.find_references(tree, self, _warning_msg=find_ref_warning_msg) if self.version <= versioning.FILL_DEFAULTS_MAX_VERSION and get_config().legacy_fill_schema_defaults: schema.fill_defaults(tree, self, reading=True) @@ -1198,13 +1205,13 @@ def write_to( if version is not None: self.version = previous_version - def find_references(self): + def find_references(self, _warning_msg=False): """ Finds all external "JSON References" in the tree and converts them to ``reference.Reference`` objects. """ # Set directly to self._tree, since it doesn't need to be re-validated. - self._tree = reference.find_references(self._tree, self) + self._tree = reference.find_references(self._tree, self, _warning_msg=_warning_msg) def resolve_references(self, **kwargs): """ diff --git a/asdf/_tests/test_deprecated.py b/asdf/_tests/test_deprecated.py index cae5e9987..0df6b1f7e 100644 --- a/asdf/_tests/test_deprecated.py +++ b/asdf/_tests/test_deprecated.py @@ -50,3 +50,20 @@ def test_resolve_and_inline_deprecation(): with pytest.warns(AsdfDeprecationWarning, match="resolve_and_inline is deprecated"): af = asdf.AsdfFile({"arr": np.arange(42)}) af.resolve_and_inline() + + +def test_find_references_during_init_deprecation(): + tree = {"a": 1, "b": {"$ref": "#"}} + with pytest.warns(AsdfDeprecationWarning, match="find_references during AsdfFile.__init__"): + asdf.AsdfFile(tree) + + +def test_find_references_during_open_deprecation(tmp_path): + fn = tmp_path / "test.asdf" + af = asdf.AsdfFile() + af["a"] = 1 + af["b"] = {"$ref": "#"} + af.write_to(fn) + with pytest.warns(AsdfDeprecationWarning, match="find_references during open"): + with asdf.open(fn) as af: + pass diff --git a/asdf/_tests/test_reference.py b/asdf/_tests/test_reference.py index d9186fa08..9e42388e0 100644 --- a/asdf/_tests/test_reference.py +++ b/asdf/_tests/test_reference.py @@ -7,6 +7,7 @@ import asdf from asdf import reference, util +from asdf.exceptions import AsdfDeprecationWarning from asdf.tags.core import ndarray from ._helpers import assert_tree_match @@ -78,16 +79,24 @@ def do_asserts(ff): assert_array_equal(ff.tree["internal"], exttree["cool_stuff"]["a"]) - with asdf.AsdfFile(tree, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) as ff: + with asdf.AsdfFile({}, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) as ff: + # avoid passing tree to AsdfFile to avoid the deprecation warning, this can be updated + # when automatic find_references on AsdfFile.__init__ is removed + ff.tree = tree + ff.find_references() do_asserts(ff) internal_path = os.path.join(str(tmp_path), "main.asdf") ff.write_to(internal_path) - with asdf.open(internal_path) as ff: + with pytest.warns(AsdfDeprecationWarning, match="find_references during open"), asdf.open(internal_path) as ff: + # this can be updated to add a find_references call when the deprecated automatic + # find_references on open is removed do_asserts(ff) - with asdf.open(internal_path) as ff: + with pytest.warns(AsdfDeprecationWarning, match="find_references during open"), asdf.open(internal_path) as ff: + # this can be updated to add a find_references call when the deprecated automatic + # find_references on open is removed assert len(ff._external_asdf_by_uri) == 0 ff.resolve_references() assert len(ff._external_asdf_by_uri) == 2 @@ -115,16 +124,28 @@ def do_asserts(ff): def test_external_reference_invalid(tmp_path): tree = {"foo": {"$ref": "fail.asdf"}} - ff = asdf.AsdfFile(tree) + # avoid passing tree to AsdfFile to avoid the deprecation warning, this can be updated + # when automatic find_references on AsdfFile.__init__ is removed + ff = asdf.AsdfFile() + ff.tree = tree + ff.find_references() with pytest.raises(ValueError, match=r"Resolved to relative URL"): ff.resolve_references() - ff = asdf.AsdfFile(tree, uri="http://httpstat.us/404") + # avoid passing tree to AsdfFile to avoid the deprecation warning, this can be updated + # when automatic find_references on AsdfFile.__init__ is removed + ff = asdf.AsdfFile({}, uri="http://httpstat.us/404") + ff.tree = tree + ff.find_references() msg = r"[HTTP Error 404: Not Found, HTTP Error 502: Bad Gateway]" # if httpstat.us is down 502 is returned. with pytest.raises(IOError, match=msg): ff.resolve_references() - ff = asdf.AsdfFile(tree, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) + # avoid passing tree to AsdfFile to avoid the deprecation warning, this can be updated + # when automatic find_references on AsdfFile.__init__ is removed + ff = asdf.AsdfFile({}, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) + ff.tree = tree + ff.find_references() with pytest.raises(IOError, match=r"No such file or directory: .*"): ff.resolve_references() @@ -137,19 +158,23 @@ def test_external_reference_invalid_fragment(tmp_path): tree = {"foo": {"$ref": "external.asdf#/list_of_stuff/a"}} - with asdf.AsdfFile(tree, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) as ff, pytest.raises( - ValueError, - match=r"Unresolvable reference: .*", - ): - ff.resolve_references() + # avoid passing tree to AsdfFile to avoid the deprecation warning, this can be updated + # when automatic find_references on AsdfFile.__init__ is removed + with asdf.AsdfFile({}, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) as ff: + ff.tree = tree + ff.find_references() + with pytest.raises(ValueError, match=r"Unresolvable reference: .*"): + ff.resolve_references() tree = {"foo": {"$ref": "external.asdf#/list_of_stuff/3"}} - with asdf.AsdfFile(tree, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) as ff, pytest.raises( - ValueError, - match=r"Unresolvable reference: .*", - ): - ff.resolve_references() + # avoid passing tree to AsdfFile to avoid the deprecation warning, this can be updated + # when automatic find_references on AsdfFile.__init__ is removed + with asdf.AsdfFile({}, uri=util.filepath_to_url(os.path.join(str(tmp_path), "main.asdf"))) as ff: + ff.tree = tree + ff.find_references() + with pytest.raises(ValueError, match=r"Unresolvable reference: .*"): + ff.resolve_references() def test_make_reference(tmp_path): @@ -169,7 +194,11 @@ def test_make_reference(tmp_path): ff.write_to(os.path.join(str(tmp_path), "source.asdf")) - with asdf.open(os.path.join(str(tmp_path), "source.asdf")) as ff: + with pytest.warns(AsdfDeprecationWarning, match="find_references during open"), asdf.open( + os.path.join(str(tmp_path), "source.asdf") + ) as ff: + # this can be updated to add a find_references call when the deprecated automatic + # find_references on open is removed assert ff.tree["ref"]._uri == "external.asdf#f~0o~0o~1/a" @@ -178,7 +207,8 @@ def test_internal_reference(tmp_path): tree = {"foo": 2, "bar": {"$ref": "#"}} - ff = asdf.AsdfFile(tree) + with pytest.warns(AsdfDeprecationWarning, match="find_references during AsdfFile.__init__"): + ff = asdf.AsdfFile(tree) ff.find_references() assert isinstance(ff.tree["bar"], reference.Reference) ff.resolve_references() diff --git a/asdf/reference.py b/asdf/reference.py index 6d4250c5c..b408653e0 100644 --- a/asdf/reference.py +++ b/asdf/reference.py @@ -5,6 +5,7 @@ """ +import warnings import weakref from collections.abc import Sequence from contextlib import suppress @@ -12,6 +13,7 @@ import numpy as np from . import generic_io, treeutil, util +from .exceptions import AsdfDeprecationWarning from .util import _patched_urllib_parse __all__ = ["resolve_fragment", "Reference", "find_references", "resolve_references", "make_reference"] @@ -104,7 +106,7 @@ def __contains__(self, item): return item in self._get_target() -def find_references(tree, ctx): +def find_references(tree, ctx, _warning_msg=False): """ Find all of the JSON references in the tree, and convert them into `Reference` objects. @@ -112,6 +114,8 @@ def find_references(tree, ctx): def do_find(tree, json_id): if isinstance(tree, dict) and "$ref" in tree: + if _warning_msg: + warnings.warn(_warning_msg, AsdfDeprecationWarning) return Reference(tree["$ref"], json_id, asdffile=ctx) return tree From 50b4ab9038b9d6af12570c74215d40fcf6dd94db Mon Sep 17 00:00:00 2001 From: Brett Date: Sat, 16 Dec 2023 12:11:33 -0500 Subject: [PATCH 2/2] add deprecation to docs --- CHANGES.rst | 3 +++ docs/asdf/deprecations.rst | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index cbced16f5..d713713c7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,6 +9,9 @@ The ASDF Standard is at v1.6.0 - Deprecate ``asdf.asdf`` and ``AsdfFile.resolve_and_inline`` [#1690] +- Deprecate automatic calling of ``AsdfFile.find_references`` during + ``AsdfFile.__init__`` and ``asdf.open`` [#1708] + 3.0.1 (2023-10-30) ------------------ diff --git a/docs/asdf/deprecations.rst b/docs/asdf/deprecations.rst index f5f7c7caf..547ac9296 100644 --- a/docs/asdf/deprecations.rst +++ b/docs/asdf/deprecations.rst @@ -16,6 +16,10 @@ Version 3.1 ``AsdfFile.resolve_references`` and provide ``all_array_storage='inline'`` to ``AdsfFile.write_to`` (or ``AsdfFile.update``). +Automatic calling of ``AsdfFile.find_references`` during calls to +``AsdfFile.__init__`` and ``asdf.open``. Call ``AsdfFile.find_references`` to +find references. + Version 3.0 ===========