Skip to content

Commit

Permalink
deprecate find_references calls during AsdfFile.__init__ and open
Browse files Browse the repository at this point in the history
  • Loading branch information
braingram committed Dec 16, 2023
1 parent 9f1d52a commit 4330bd5
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 24 deletions.
17 changes: 12 additions & 5 deletions asdf/_asdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 = []

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
"""
Expand Down
17 changes: 17 additions & 0 deletions asdf/_tests/test_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
66 changes: 48 additions & 18 deletions asdf/_tests/test_reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand All @@ -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):
Expand All @@ -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"


Expand All @@ -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()
Expand Down
6 changes: 5 additions & 1 deletion asdf/reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
"""


import warnings
import weakref
from collections.abc import Sequence
from contextlib import suppress

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"]
Expand Down Expand Up @@ -104,14 +106,16 @@ 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.
"""

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

Expand Down

0 comments on commit 4330bd5

Please sign in to comment.