Skip to content

Commit

Permalink
Merge pull request #1853 from braingram/err_select_tag
Browse files Browse the repository at this point in the history
Error for Convert subclasses with multiple tags if select_tag isn't implemented
  • Loading branch information
braingram authored Oct 31, 2024
2 parents f3f6070 + 583034e commit 8d9b721
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 23 deletions.
6 changes: 2 additions & 4 deletions asdf/_tests/test_extension.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import asdf
from asdf import AsdfFile, config_context
from asdf.exceptions import AsdfManifestURIMismatchWarning, AsdfSerializationError, AsdfWarning, ValidationError
from asdf.exceptions import AsdfManifestURIMismatchWarning, AsdfSerializationError, ValidationError
from asdf.extension import (
Compressor,
Converter,
Expand Down Expand Up @@ -892,10 +892,8 @@ def from_yaml_tree(self, node, tag, ctx):
"asdf://somewhere.org/tags/foo-2.0.0",
]
extension = FullExtension(converters=[FooConverter()], tags=tags)
ctx_type = pytest.warns if is_subclass else pytest.raises
exception_class = AsdfWarning if is_subclass else RuntimeError
with config_context() as config:
with ctx_type(exception_class, match="Converter handles multiple tags"):
with pytest.raises(RuntimeError, match="Converter handles multiple tags"):
config.add_extension(extension)


Expand Down
21 changes: 2 additions & 19 deletions asdf/extension/_converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
"""

import abc
import warnings

from asdf.exceptions import AsdfWarning
from asdf.util import get_class_name, uri_match


Expand Down Expand Up @@ -179,23 +177,8 @@ def __init__(self, delegate, extension):
raise TypeError(msg)

if len(relevant_tags) > 1 and not hasattr(delegate, "select_tag"):
# we cannot use isinstance here because Converter supports
# virtual subclasses
if Converter in delegate.__class__.__mro__:
# prior to asdf 3.0 Converter provided a default select_tag
# to provide backwards compatibility allow Converter subclasses
# to be registered with >1 tag but produce a warning
msg = (
"Converter handles multiple tags for this extension, "
"but does not implement a select_tag method. "
"This previously worked because Converter subclasses inherited "
"the now removed select_tag. This will be an error in a future "
"version of asdf"
)
warnings.warn(msg, AsdfWarning)
else:
msg = "Converter handles multiple tags for this extension, but does not implement a select_tag method."
raise RuntimeError(msg)
msg = "Converter handles multiple tags for this extension, but does not implement a select_tag method."
raise RuntimeError(msg)

self._tags = sorted(relevant_tags)

Expand Down
1 change: 1 addition & 0 deletions changes/1853.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Raise RuntimeError if a Convert subclass supports multiple tags but doesn't implement select_tag.

0 comments on commit 8d9b721

Please sign in to comment.