Add warning when conversion fails with default warn_on_failed_conversion

asdf/_helpers.py

@@ -1,3 +1,6 @@
+from functools import cached_property
+from typing import Any
+
 from . import versioning
 from ._version import version as asdf_package_version
 
@@ -13,3 +16,31 @@ def validate_version(version):
         )
         raise ValueError(msg)
     return version
+
+
+class _IsSet:
+    """Base class that tracks when its attributes are set.
+
+    Attributes are only considered set when they have been *re-assigned* after initially being created.
+    """
+
+    @cached_property
+    def _is_set(self):
+        return set()
+
+    def __setattr__(self, name: str, value: Any) -> None:
+        if hasattr(self, name):
+            self._is_set.add(name)
+
+        super().__setattr__(name, value)
+
+
+def is_set(obj: _IsSet, attr: str) -> bool:
+    """Get whether `attr` has been set on `obj`."""
+    if not isinstance(obj, _IsSet):
+        msg = f"Object with type {type(obj).__name__} is not an instance of _IsSet"
+        raise TypeError(msg)
+    if not hasattr(obj, attr):
+        raise AttributeError(attr)
+
+    return attr in obj._is_set

asdf/_tests/tags/core/tests/test_ndarray.py

@@ -11,7 +11,7 @@
 from numpy.testing import assert_array_equal
 
 import asdf
-from asdf.exceptions import ValidationError
+from asdf.exceptions import AsdfFutureWarning, ValidationError
 from asdf.extension import Converter, Extension, TagDefinition
 from asdf.tags.core import ndarray
 from asdf.testing import helpers
@@ -954,8 +954,9 @@ def test_inline_shape_mismatch(ndarray_tag):
 
     buff = helpers.yaml_to_asdf(content)
     with pytest.raises(ValueError, match=r"inline data doesn't match the given shape"):
-        with asdf.open(buff) as af:
-            af["arr"]
+        with pytest.warns(AsdfFutureWarning):
+            with asdf.open(buff) as af:
+                af["arr"]
 
 
 def test_broadcasted_array():

asdf/_tests/test_config.py

@@ -6,8 +6,11 @@
 import asdf
 from asdf import get_config
 from asdf._core._integration import get_json_schema_resource_mappings
+from asdf.config import config_context
+from asdf.exceptions import AsdfConversionWarning, AsdfFutureWarning
 from asdf.extension import ExtensionProxy
 from asdf.resource import ResourceMappingProxy
+from asdf.testing import helpers
 
 
 def test_config_context():
@@ -353,3 +356,77 @@ def test_invalid_set_default_array_save_base(value):
     with asdf.config_context() as config:
         with pytest.raises(ValueError, match="default_array_save_base must be a bool"):
             config.default_array_save_base = value
+
+
+class NeverConverter:
+    tags = ["asdf://example.com/tags/never-1.0.0"]
+    types = []
+    lazy = True
+
+    def to_yaml_tree(self, obj, tag, ctx):
+        return {}
+
+    def from_yaml_tree(self, node, tag, ctx):
+        msg = "This type always fails"
+        raise RuntimeError(msg)
+
+
+class NeverExtension:
+    tags = NeverConverter.tags
+    extension_uri = "asdf://example.com/extensions/never-1.0.0"
+    converters = [NeverConverter()]
+
+
+@pytest.mark.parametrize("lazy", [True, False])
+def test_warn_on_failed_conversion_default_warn(lazy: bool):
+    """Test that when a node conversion fails a FutureWarning is emitted before the error
+    if warn_on_failed_conversion hasn't been manually set.
+
+    This test can be removed once the default for warn_on_failed_conversion changes.
+    """
+    with config_context() as cfg:
+        cfg.lazy_tree = lazy
+        cfg.add_extension(NeverExtension())
+        buff = helpers.yaml_to_asdf(f"data: !<{NeverConverter.tags[0]}> {{}}")
+
+        with pytest.raises(RuntimeError), pytest.warns(AsdfFutureWarning):
+            with asdf.open(buff) as af:
+                af["data"]
+
+
+@pytest.mark.filterwarnings("error:AsdfFutureWarning")
+@pytest.mark.parametrize("lazy", [True, False])
+def test_warn_on_failed_conversion_false_no_warn(lazy: bool):
+    """Test that when a node conversion fails no extra warning is emitted if
+    warn_on_failed_conversion has been set to False.
+
+    This test can be removed once the default for warn_on_failed_conversion changes.
+    """
+    with config_context() as cfg:
+        cfg.lazy_tree = lazy
+        cfg.add_extension(NeverExtension())
+        buff = helpers.yaml_to_asdf(f"data: !<{NeverConverter.tags[0]}> {{}}")
+
+        cfg.warn_on_failed_conversion = False
+        with pytest.raises(RuntimeError):
+            with asdf.open(buff) as af:
+                af["data"]
+
+
+@pytest.mark.filterwarnings("error:AsdfFutureWarning")
+@pytest.mark.parametrize("lazy", [True, False])
+def test_warn_on_failed_conversion_true_no_warn(lazy: bool):
+    """Test that when a node conversion fails no extra warning is emitted if
+    warn_on_failed_conversion has been set to True.
+
+    This test can be removed once the default for warn_on_failed_conversion changes.
+    """
+    with config_context() as cfg:
+        cfg.lazy_tree = lazy
+        cfg.add_extension(NeverExtension())
+        buff = helpers.yaml_to_asdf(f"data: !<{NeverConverter.tags[0]}> {{}}")
+
+        cfg.warn_on_failed_conversion = True
+        with pytest.warns(AsdfConversionWarning):
+            with asdf.open(buff) as af:
+                af["data"]

asdf/_tests/test_helpers.py

@@ -0,0 +1,50 @@
+import pytest
+
+from asdf._helpers import _IsSet, is_set
+
+
+class Tracked(_IsSet):
+    bar = None
+
+    def __init__(self):
+        # Intentionally not calling `super().__init__()` here to make sure `_IsSet` still works
+        self._value = 1
+        self.baz = None
+
+    @property
+    def value(self):
+        return self._value
+
+    @value.setter
+    def value(self, value):
+        self._value = value
+
+
+def test_is_set():
+    x = Tracked()
+    y = Tracked()
+
+    def is_set_iter(obj):
+        yield from (is_set(obj, attr) for attr in ["value", "bar", "baz"])
+
+    assert not any(is_set_iter(x))
+
+    x.value = 2
+    x.bar = 3
+    x.baz = "foo"
+
+    assert all(is_set_iter(x))
+    assert not any(is_set_iter(y))
+
+
+def test_is_set_attr_error():
+    """Test that trying to access a non-existent property via `is_set` raises an `AttributeError`."""
+    x = Tracked()
+    with pytest.raises(AttributeError):
+        is_set(x, "foo")
+
+
+def test_is_set_type_error():
+    with pytest.raises(TypeError):
+        # pyrefly: ignore [bad-argument-type]
+        is_set(object(), "foo")

asdf/_tests/test_lazy_nodes.py

@@ -8,6 +8,10 @@
 import pytest
 
 import asdf
+import asdf.lazy_nodes
+import asdf.tagged
+import asdf.tags.core
+import asdf.treeutil
 from asdf.lazy_nodes import AsdfDictNode, AsdfListNode, AsdfOrderedDictNode, _resolve_af_ref, _to_lazy_node
 
 

asdf/_tests/test_util.py

@@ -5,6 +5,7 @@
 import pytest
 
 import asdf
+import asdf.tagged
 from asdf import generic_io, util
 
 

asdf/config.py

@@ -12,7 +12,7 @@
 from typing import TYPE_CHECKING, Any
 
 from . import _entry_points, util, versioning
-from ._helpers import validate_version
+from ._helpers import _IsSet, validate_version
 from .extension import ExtensionProxy
 from .resource import ResourceManager, ResourceMappingProxy
 
@@ -37,14 +37,15 @@
 DEFAULT_WARN_ON_FAILED_CONVERSION = False
 
 
-class AsdfConfig:
+class AsdfConfig(_IsSet):
     """
     Container for ASDF configuration options.  Users are not intended to
     construct this object directly; instead, use the `asdf.get_config` and
     `asdf.config_context` module methods.
     """
 
     def __init__(self):
+        super().__init__()
         self._resource_mappings = None
         self._resource_manager = None
         self._extensions = None
@@ -476,6 +477,13 @@ def warn_on_failed_conversion(self) -> bool:
         Returns
         -------
         bool
+
+        Warnings
+        --------
+        In a future release the default value will change from `False` to `True`.
+        Currently, if a conversion fails and this field hasn't been set then ASDF will
+        emit an `asdf.exceptions.AsdfFutureWarning` *before* raising an exception.
+        Manually set the field to either `True` or `False` to silence this warning.
         """
         return self._warn_on_failed_conversion
 

asdf/exceptions.py

@@ -5,6 +5,7 @@
 __all__ = [
     "AsdfConversionWarning",
     "AsdfDeprecationWarning",
+    "AsdfFutureWarning",
     "AsdfLazyReferenceError",
     "AsdfManifestURIMismatchWarning",
     "AsdfPackageVersionWarning",
@@ -84,3 +85,7 @@ class AsdfSerializationError(RepresenterError):
     that the object does not have a supporting asdf Converter and needs to
     be manually converted to a supported type.
     """
+
+
+class AsdfFutureWarning(AsdfWarning, FutureWarning):
+    """Warning about a future change in ASDF behavior."""

asdf/lazy_nodes.py

@@ -8,7 +8,10 @@
 import warnings
 import weakref
 
+from asdf.exceptions import AsdfFutureWarning
+
 from . import tagged, treeutil, yamlutil
+from ._helpers import is_set
 from .config import get_config
 from .exceptions import AsdfConversionWarning, AsdfLazyReferenceError
 from .extension._serialization_context import BlockAccess
@@ -221,6 +224,17 @@ def _convert_and_cache(self, value, key):
                             warnings.warn(f"A node failed to convert with: {err}", AsdfConversionWarning)
                             obj = _to_lazy_node(value, self._af_ref)
                         else:
+                            if not is_set(get_config(), "warn_on_failed_conversion"):
+                                # Only emit a warning if warn_on_failed_conversion hasn't been manually set
+                                # This branch can be removed once the default for warn_on_failed_conversion is changed
+                                warnings.warn(
+                                    (
+                                        "In the future failed node conversion will by default generate a warning "
+                                        "instead of an error. Set AsdfConfig.warn_on_failed_conversion to True to "
+                                        "opt-in to the new behavior, or to False to silence this warning."
+                                    ),
+                                    AsdfFutureWarning,
+                                )
                             raise
                     sctx.assign_object(obj)
                     sctx.assign_blocks()

asdf/util.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 import enum
 import importlib.util
 import math

asdf/yamlutil.py

@@ -8,7 +8,10 @@
 import numpy as np
 import yaml
 
+from asdf.exceptions import AsdfFutureWarning
+
 from . import schema, tagged, treeutil, util
+from ._helpers import is_set
 from .config import get_config
 from .constants import STSCI_SCHEMA_TAG_BASE, YAML_TAG_PREFIX
 from .exceptions import AsdfConversionWarning, AsdfSerializationError
@@ -351,6 +354,16 @@ def _walker(node):
                     warnings.warn(f"A node failed to convert with: {err}", AsdfConversionWarning)
                     obj = node
                 else:
+                    if not is_set(get_config(), "warn_on_failed_conversion"):
+                        # Only emit a warning if warn_on_failed_conversion hasn't been manually set
+                        warnings.warn(
+                            (
+                                "In the future failed node conversion will by default generate a warning "
+                                "instead of an error. Set AsdfConfig.warn_on_failed_conversion to True to opt-in "
+                                "to the new behavior, or to False to silence this warning."
+                            ),
+                            AsdfFutureWarning,
+                        )
                     raise
             _serialization_context.assign_object(obj)
             _serialization_context.assign_blocks()

changes/2125.general.rst

@@ -0,0 +1,4 @@
+In a future release the default value for `asdf.config.AsdfConfig.warn_on_failed_conversion` will change from `False` to `True`.
+Currently if ASDF raises an exception due to a conversion error it will now *also* emit a warning regarding the change in behavior.
+To silence the warning you can either set ``warn_on_failed_conversion`` to `True` to opt into the new behavior
+or to `False` to retain the old behavior.

docs/asdf/config.rst

@@ -212,6 +212,12 @@ a custom object are caught and turned into warnings. It may be helpful to
 enable this option when opening old files with tags that are no longer supported
 in the current environment.
 
+.. warning::
+  In a future release the default value will change from `False` to `True`.
+  Currently, if a conversion fails and this field hasn't been set then ASDF will
+  emit an `asdf.exceptions.AsdfFutureWarning` *before* raising an exception.
+  Manually set the field to either `True` or `False` to silence this warning.
+
 Additional AsdfConfig features
 ==============================