Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deprecate ignore_implicit_conversion and implicit_conversion #1724

Merged
merged 5 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
------------------
Expand Down
3 changes: 2 additions & 1 deletion asdf/_asdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions asdf/_tests/test_deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
61 changes: 44 additions & 17 deletions asdf/_tests/test_yaml.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import contextlib
import io
import re
from collections import OrderedDict, namedtuple
from typing import NamedTuple

Expand All @@ -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
----------
Expand All @@ -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 = {
Expand Down Expand Up @@ -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():
Expand All @@ -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.
Expand All @@ -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():
Expand All @@ -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()


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


Expand 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")
Expand Down
11 changes: 9 additions & 2 deletions asdf/treeutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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``.
Expand All @@ -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"
Expand Down Expand Up @@ -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

Expand Down
15 changes: 13 additions & 2 deletions docs/asdf/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
===========

Expand Down Expand Up @@ -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
===========

Expand Down
Loading