From 93c81a2543621951c4d72a6c56695b1013ba36a3 Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 3 Jan 2024 16:18:04 -0500 Subject: [PATCH 1/5] deprecate ignore_implicit_conversion and implicit_conversion --- asdf/_asdf.py | 3 ++- asdf/_tests/test_deprecated.py | 13 ++++++++++++ asdf/_tests/test_yaml.py | 37 ++++++++++++++++++++++++++-------- asdf/treeutil.py | 11 ++++++++-- docs/asdf/deprecations.rst | 15 ++++++++++++-- 5 files changed, 66 insertions(+), 13 deletions(-) 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..c3a1441fd 100644 --- a/asdf/_tests/test_deprecated.py +++ b/asdf/_tests/test_deprecated.py @@ -82,6 +82,7 @@ def test_find_references_during_open_deprecation(tmp_path): pass +<<<<<<< HEAD def test_asdf_util_is_primitive_deprecation(): with pytest.warns(AsdfDeprecationWarning, match="asdf.util.is_primitive is deprecated"): asdf.util.is_primitive(1) @@ -134,3 +135,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..e5fe995e2 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,7 +10,7 @@ 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 @@ -118,7 +120,23 @@ def test_python_tuple(): input_tree = {"val": (1, 2, 3)} - run_tuple_test(input_tree) + with pytest.warns(AsdfDeprecationWarning, match="ignore_implicit_conversion"): + 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(): @@ -130,7 +148,7 @@ def test_named_tuple_collections(): input_tree = {"val": nt(1, 2, 3)} - with pytest.warns(AsdfWarning, match="converting to list"): + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): run_tuple_test(input_tree) @@ -146,7 +164,7 @@ class NT(NamedTuple): tree = {"val": NT(1, 2, 3)} - with pytest.warns(AsdfWarning, match="converting to list"): + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): run_tuple_test(tree) @@ -155,7 +173,8 @@ def test_named_tuple_collections_recursive(): input_tree = {"val": nt(1, 2, np.ones(3))} - with pytest.warns(AsdfWarning, match="converting to list"): + #init_options = {"ignore_implicit_conversion": True} + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): _, tree = _roundtrip(input_tree) assert (tree["val"][2] == np.ones(3)).all() @@ -168,7 +187,8 @@ class NT(NamedTuple): input_tree = {"val": NT(1, 2, np.ones(3))} - with pytest.warns(AsdfWarning, match="converting to list"): + init_options = {"ignore_implicit_conversion": True} + with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): _, tree = _roundtrip(input_tree) assert (tree["val"][2] == np.ones(3)).all() @@ -181,8 +201,9 @@ def test_implicit_conversion_warning(): with pytest.warns(AsdfWarning, match=r"Failed to serialize instance"), 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 =========== From 48b0f226df67a53457c995906502a47eb8c5b72a Mon Sep 17 00:00:00 2001 From: Brett Date: Wed, 3 Jan 2024 16:20:59 -0500 Subject: [PATCH 2/5] add changelog --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) 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) ------------------ From 741e84d082197dac10f136f71d0f53041bf45678 Mon Sep 17 00:00:00 2001 From: Brett Date: Tue, 27 Feb 2024 08:22:56 -0500 Subject: [PATCH 3/5] fix rebase error --- asdf/_tests/test_deprecated.py | 1 - 1 file changed, 1 deletion(-) diff --git a/asdf/_tests/test_deprecated.py b/asdf/_tests/test_deprecated.py index c3a1441fd..810a8d4c9 100644 --- a/asdf/_tests/test_deprecated.py +++ b/asdf/_tests/test_deprecated.py @@ -82,7 +82,6 @@ def test_find_references_during_open_deprecation(tmp_path): pass -<<<<<<< HEAD def test_asdf_util_is_primitive_deprecation(): with pytest.warns(AsdfDeprecationWarning, match="asdf.util.is_primitive is deprecated"): asdf.util.is_primitive(1) From 8294883ec17a96433fecb39200b539fd48254101 Mon Sep 17 00:00:00 2001 From: Brett Date: Tue, 27 Feb 2024 08:37:03 -0500 Subject: [PATCH 4/5] update test for new pytest.warns behavior --- asdf/_tests/test_yaml.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/asdf/_tests/test_yaml.py b/asdf/_tests/test_yaml.py index e5fe995e2..12d309685 100644 --- a/asdf/_tests/test_yaml.py +++ b/asdf/_tests/test_yaml.py @@ -198,7 +198,11 @@ 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 multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): From 620c9c5a27febf3d63273ca9f6053e7aa34b56db Mon Sep 17 00:00:00 2001 From: Brett Date: Fri, 10 May 2024 10:51:03 -0400 Subject: [PATCH 5/5] update tests for recent changes --- asdf/_tests/test_yaml.py | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/asdf/_tests/test_yaml.py b/asdf/_tests/test_yaml.py index 12d309685..34598420c 100644 --- a/asdf/_tests/test_yaml.py +++ b/asdf/_tests/test_yaml.py @@ -14,7 +14,7 @@ from asdf.testing.helpers import yaml_to_asdf -def _roundtrip(obj): +def _roundtrip(obj, init_kwargs=None): """ Parameters ---------- @@ -31,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 = { @@ -104,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(): @@ -120,8 +125,7 @@ def test_python_tuple(): input_tree = {"val": (1, 2, 3)} - with pytest.warns(AsdfDeprecationWarning, match="ignore_implicit_conversion"): - run_tuple_test(input_tree) + run_tuple_test(input_tree) @contextlib.contextmanager @@ -149,7 +153,7 @@ def test_named_tuple_collections(): input_tree = {"val": nt(1, 2, 3)} with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): - run_tuple_test(input_tree) + run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) def test_named_tuple_typing(): @@ -162,10 +166,10 @@ class NT(NamedTuple): two: int three: int - tree = {"val": NT(1, 2, 3)} + input_tree = {"val": NT(1, 2, 3)} with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): - run_tuple_test(tree) + run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) def test_named_tuple_collections_recursive(): @@ -173,9 +177,8 @@ def test_named_tuple_collections_recursive(): input_tree = {"val": nt(1, 2, np.ones(3))} - #init_options = {"ignore_implicit_conversion": True} with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): - _, tree = _roundtrip(input_tree) + _, tree = run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) assert (tree["val"][2] == np.ones(3)).all() @@ -187,9 +190,8 @@ class NT(NamedTuple): input_tree = {"val": NT(1, 2, np.ones(3))} - init_options = {"ignore_implicit_conversion": True} with multi_warn(AsdfDeprecationWarning, ["ignore_implicit_conversion", "implicit conversion is deprecated"]): - _, tree = _roundtrip(input_tree) + _, tree = run_tuple_test(input_tree, init_kwargs={"ignore_implicit_conversion": True}) assert (tree["val"][2] == np.ones(3)).all()