Merge pull request #1724 from braingram/named_tuple

deprecate ignore_implicit_conversion and implicit_conversion

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)
 ------------------

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

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)

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,19 +166,19 @@ 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():
     nt = namedtuple("TestNamedTuple3", ("one", "two", "three"))
 
     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")

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
 

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
 ===========