diff --git a/CHANGES.rst b/CHANGES.rst index 14d855d1c..a59642e7e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -29,6 +29,8 @@ ``AsdfFile.set_array_save_base`` and ``SerializationContext.set_array_save_base`` [#1753] +- Deprecate ``ignore_implicit_conversion`` and "implicit conversion" [#1724] + 3.2.0 (2024-04-05) ------------------ diff --git a/asdf/_asdf.py b/asdf/_asdf.py index 38541a6d2..4afaedd0f 100644 --- a/asdf/_asdf.py +++ b/asdf/_asdf.py @@ -74,7 +74,7 @@ def __init__( version=None, ignore_version_mismatch=True, ignore_unrecognized_tag=False, - ignore_implicit_conversion=False, + ignore_implicit_conversion=NotSet, copy_arrays=False, memmap=NotSet, lazy_load=True, @@ -110,6 +110,7 @@ def __init__( `False` by default. ignore_implicit_conversion : bool + DEPRECATED When `True`, do not raise warnings when types in the tree are implicitly converted into a serializable object. The motivating case for this is currently ``namedtuple``, which cannot be serialized diff --git a/asdf/_tests/test_deprecated.py b/asdf/_tests/test_deprecated.py index 03568b681..810a8d4c9 100644 --- a/asdf/_tests/test_deprecated.py +++ b/asdf/_tests/test_deprecated.py @@ -134,3 +134,15 @@ def test_format_tag_deprecation(): def test_asdf_util_filepath_to_url_deprecation(tmp_path): with pytest.warns(AsdfDeprecationWarning, match="asdf.util.filepath_to_url is deprecated"): asdf.util.filepath_to_url(str(tmp_path)) + + +@pytest.mark.parametrize("value", [True, False]) +def test_AsdfFile_ignore_implicit_conversion_deprecation(value): + with pytest.warns(AsdfDeprecationWarning, match="ignore_implicit_conversion is deprecated"): + asdf.AsdfFile({"a": 1}, ignore_implicit_conversion=value) + + +@pytest.mark.parametrize("value", [True, False]) +def test_walk_and_modify_ignore_implicit_conversion_deprecation(value): + with pytest.warns(AsdfDeprecationWarning, match="ignore_implicit_conversion is deprecated"): + asdf.treeutil.walk_and_modify({}, lambda obj: obj, ignore_implicit_conversion=value) diff --git a/asdf/_tests/test_yaml.py b/asdf/_tests/test_yaml.py index 4438bfb69..34598420c 100644 --- a/asdf/_tests/test_yaml.py +++ b/asdf/_tests/test_yaml.py @@ -1,4 +1,6 @@ +import contextlib import io +import re from collections import OrderedDict, namedtuple from typing import NamedTuple @@ -8,11 +10,11 @@ import asdf from asdf import tagged, treeutil, yamlutil -from asdf.exceptions import AsdfConversionWarning, AsdfWarning +from asdf.exceptions import AsdfConversionWarning, AsdfDeprecationWarning, AsdfWarning from asdf.testing.helpers import yaml_to_asdf -def _roundtrip(obj): +def _roundtrip(obj, init_kwargs=None): """ Parameters ---------- @@ -29,8 +31,12 @@ def _roundtrip(obj): read_tree : object object read back from ASDF file """ + + init_kwargs = init_kwargs or {} buff = io.BytesIO() - asdf.AsdfFile({"obj": obj}).write_to(buff) + af = asdf.AsdfFile(**init_kwargs) + af["obj"] = obj + af.write_to(buff) buff.seek(0) open_kwargs = { @@ -102,11 +108,12 @@ class Foo: ff.write_to(buff) -def run_tuple_test(input_tree): - content, tree = _roundtrip(input_tree) +def run_tuple_test(input_tree, **kwargs): + content, tree = _roundtrip(input_tree, **kwargs) assert b"tuple" not in content assert isinstance(tree["val"], list) + return content, tree def test_python_tuple(): @@ -121,6 +128,21 @@ def test_python_tuple(): run_tuple_test(input_tree) +@contextlib.contextmanager +def multi_warn(category, matches): + with pytest.warns(category) as record: + yield + + for match in matches: + found = False + for r in record: + msg = str(r.message) + if re.match(match, msg): + found = True + break + assert found, f"Did not raise {category} with message matching {match}" + + def test_named_tuple_collections(): """ Ensure that we are able to serialize a collections.namedtuple. @@ -130,8 +152,8 @@ def test_named_tuple_collections(): input_tree = {"val": nt(1, 2, 3)} - with pytest.warns(AsdfWarning, match="converting to list"): - run_tuple_test(input_tree) + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): + run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) def test_named_tuple_typing(): @@ -144,10 +166,10 @@ class NT(NamedTuple): two: int three: int - tree = {"val": NT(1, 2, 3)} + input_tree = {"val": NT(1, 2, 3)} - with pytest.warns(AsdfWarning, match="converting to list"): - run_tuple_test(tree) + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): + run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) def test_named_tuple_collections_recursive(): @@ -155,8 +177,8 @@ def test_named_tuple_collections_recursive(): input_tree = {"val": nt(1, 2, np.ones(3))} - with pytest.warns(AsdfWarning, match="converting to list"): - _, tree = _roundtrip(input_tree) + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): + _, tree = run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) assert (tree["val"][2] == np.ones(3)).all() @@ -168,8 +190,8 @@ class NT(NamedTuple): input_tree = {"val": NT(1, 2, np.ones(3))} - with pytest.warns(AsdfWarning, match="converting to list"): - _, tree = _roundtrip(input_tree) + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): + _, tree = run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) assert (tree["val"][2] == np.ones(3)).all() @@ -178,11 +200,16 @@ def test_implicit_conversion_warning(): tree = {"val": nt(1, 2, np.ones(3))} - with pytest.warns(AsdfWarning, match=r"Failed to serialize instance"), asdf.AsdfFile(tree): + with ( + pytest.warns(AsdfWarning, match=r"Failed to serialize instance"), + pytest.warns(AsdfDeprecationWarning, match=r"implicit conversion is deprecated"), + asdf.AsdfFile(tree), + ): pass - with asdf.AsdfFile(tree, ignore_implicit_conversion=True): - pass + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): + with asdf.AsdfFile(tree, ignore_implicit_conversion=True): + pass @pytest.mark.xfail(reason="pyyaml has a bug and does not support tuple keys") diff --git a/asdf/treeutil.py b/asdf/treeutil.py index 27b2989cb..4b562d039 100644 --- a/asdf/treeutil.py +++ b/asdf/treeutil.py @@ -7,7 +7,8 @@ from contextlib import contextmanager from . import tagged -from .exceptions import AsdfWarning +from .exceptions import AsdfDeprecationWarning, AsdfWarning +from .util import NotSet __all__ = ["walk", "iter_tree", "walk_and_modify", "get_children", "is_container", "PendingValue", "RemoveNode"] @@ -220,7 +221,7 @@ def __repr__(self): RemoveNode = _RemoveNode() -def walk_and_modify(top, callback, ignore_implicit_conversion=False, postorder=True, _context=None): +def walk_and_modify(top, callback, ignore_implicit_conversion=NotSet, postorder=True, _context=None): """Modify a tree by walking it with a callback function. It also has the effect of doing a deep copy. @@ -253,6 +254,7 @@ def walk_and_modify(top, callback, ignore_implicit_conversion=False, postorder=T parents first. Defaults to `True`. ignore_implicit_conversion : bool + DEPRECATED Controls whether warnings should be issued when implicitly converting a given type instance in the tree into a serializable object. The primary case for this is currently ``namedtuple``. @@ -265,6 +267,10 @@ def walk_and_modify(top, callback, ignore_implicit_conversion=False, postorder=T The modified tree. """ + if ignore_implicit_conversion is NotSet: + ignore_implicit_conversion = False + else: + warnings.warn("ignore_implicit_conversion is deprecated", AsdfDeprecationWarning) callback_arity = callback.__code__.co_argcount if callback_arity < 1 or callback_arity > 2: msg = "Expected callback to accept one or two arguments" @@ -359,6 +365,7 @@ def _handle_immutable_sequence(node, json_id): if not ignore_implicit_conversion: warnings.warn(f"Failed to serialize instance of {type(node)}, converting to list instead", AsdfWarning) result = contents + warnings.warn("implicit conversion is deprecated. Please instead use a Converter.", AsdfDeprecationWarning) return result diff --git a/docs/asdf/deprecations.rst b/docs/asdf/deprecations.rst index 443831cf2..c3d83f3a7 100644 --- a/docs/asdf/deprecations.rst +++ b/docs/asdf/deprecations.rst @@ -6,6 +6,19 @@ Deprecations ************ +Version 3.3 +=========== + +``asdf.util.filepath_to_url`` is deprecated. Please use ``pathlib.Path.to_uri``. + +The ``ignore_implicit_conversion`` argument for ``AsdfFile`` and +``treeutil.walk_and_modify`` is deprecated. "implicit conversion" is also +deprecated. This referred to the behavior where certain types (namedtuple) +were silently (or with a warning depending on the ``ignore_implicit_conversion`` +setting) converted to a list when added to an asdf tree. As these types +(namedtuple) can be supported by a ``Converter`` this "implicit conversion" +will be removed. + Version 3.1 =========== @@ -33,8 +46,6 @@ be issued on a failed validation during the following methods: Providing ``kwargs`` to ``AsdfFile.resolve_references`` does nothing and is deprecated. -``asdf.util.filepath_to_url`` is deprecated. Please use ``pathlib.Path.to_uri``. - Version 3.0 ===========