From 7c1c5aa67148a76ac5ff5f2b183a9fc81756cad9 Mon Sep 17 00:00:00 2001 From: Vadim Markovtsev Date: Wed, 22 Aug 2018 13:42:38 +0200 Subject: [PATCH 1/5] Allow typing.NamedTuple to be serialized Signed-off-by: Vadim Markovtsev --- asdf/tests/test_yaml.py | 67 +++++++++++++++++++++++++++++++++++++---- asdf/treeutil.py | 18 ++++++++--- setup.cfg | 2 +- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/asdf/tests/test_yaml.py b/asdf/tests/test_yaml.py index a374383fe..3e739f7fc 100644 --- a/asdf/tests/test_yaml.py +++ b/asdf/tests/test_yaml.py @@ -2,7 +2,8 @@ # -*- coding: utf-8 -*- import io -from collections import OrderedDict +from collections import namedtuple, OrderedDict +from typing import NamedTuple import numpy as np @@ -88,6 +89,17 @@ class Foo(object): ff.write_to(buff) +def run_tuple_test(tree, tmpdir): + def check_asdf(asdf): + assert isinstance(asdf.tree['val'], list) + + def check_raw_yaml(content): + assert b'tuple' not in content + + helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf, + raw_yaml_check_func=check_raw_yaml) + + def test_python_tuple(tmpdir): # We don't want to store tuples as tuples, because that's not a # built-in YAML data type. This test ensures that they are @@ -97,14 +109,57 @@ def test_python_tuple(tmpdir): "val": (1, 2, 3) } + run_tuple_test(tree, tmpdir) + + +def test_named_tuple_collections(tmpdir): + # Ensure that we are able to serialize a collections.namedtuple. + + nt = namedtuple("TestNamedTuple1", ("one", "two", "three")) + + tree = { + "val": nt(1, 2, 3) + } + + run_tuple_test(tree, tmpdir) + +def test_named_tuple_typing(tmpdir): + # Ensure that we are able to serialize a typing.NamedTuple. + + nt = NamedTuple("TestNamedTuple2", + (("one", int), ("two", int), ("three", int))) + tree = { + "val": nt(1, 2, 3) + } + + run_tuple_test(tree, tmpdir) + + +def test_named_tuple_collections_recursive(tmpdir): + nt = namedtuple("TestNamedTuple3", ("one", "two", "three")) + + tree = { + "val": nt(1, 2, np.ones(3)) + } + def check_asdf(asdf): - assert isinstance(asdf.tree['val'], list) + assert (asdf.tree['val'][2] == np.ones(3)).all() - def check_raw_yaml(content): - assert b'tuple' not in content + helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf) - helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf, - raw_yaml_check_func=check_raw_yaml) + +def test_named_tuple_typing_recursive(tmpdir): + nt = NamedTuple("TestNamedTuple4", + (("one", int), ("two", int), ("three", np.ndarray))) + + tree = { + "val": nt(1, 2, np.ones(3)) + } + + def check_asdf(asdf): + assert (asdf.tree['val'][2] == np.ones(3)).all() + + helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf) def test_tags_removed_after_load(tmpdir): diff --git a/asdf/treeutil.py b/asdf/treeutil.py index f782f4e70..66a01ed5d 100644 --- a/asdf/treeutil.py +++ b/asdf/treeutil.py @@ -134,8 +134,13 @@ def recurse(tree): result = tag_object(tree._tag, result) elif isinstance(tree, (list, tuple)): seen.add(id_tree) - result = tree.__class__( - [recurse(val) for val in tree]) + contents = [recurse(val) for val in tree] + try: + result = tree.__class__(contents) + except TypeError: + # the derived class' signature is different + # erase the type + result = contents seen.remove(id_tree) if hasattr(tree, '_tag'): result = tag_object(tree._tag, result) @@ -166,8 +171,13 @@ def recurse_with_json_ids(tree, json_id): result = tag_object(tree._tag, result) elif isinstance(tree, (list, tuple)): seen.add(id_tree) - result = tree.__class__( - [recurse_with_json_ids(val, json_id) for val in tree]) + contents = [recurse_with_json_ids(val, json_id) for val in tree] + try: + result = tree.__class__(contents) + except TypeError: + # the derived class' signature is different + # erase the type + result = contents seen.remove(id_tree) if hasattr(tree, '_tag'): result = tag_object(tree._tag, result) diff --git a/setup.cfg b/setup.cfg index 40aa5255e..9b239808d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,7 +17,7 @@ open_files_ignore = test.fits asdf.fits # Account for both the astropy test runner case and the native pytest case asdf_schema_root = asdf-standard/schemas asdf/schemas asdf_schema_skip_names = asdf-schema-1.0.0 draft-01 -addopts = --doctest-rst +#addopts = --doctest-rst [ah_bootstrap] auto_use = True From 6ae6796f665a77202e4348e8c293597d2a105080 Mon Sep 17 00:00:00 2001 From: Daniel D'Avella Date: Wed, 29 Aug 2018 09:21:25 -0400 Subject: [PATCH 2/5] Add warning when converting namedtuple to list --- asdf/treeutil.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/asdf/treeutil.py b/asdf/treeutil.py index 66a01ed5d..df47d9fba 100644 --- a/asdf/treeutil.py +++ b/asdf/treeutil.py @@ -5,6 +5,7 @@ """ import inspect +import warnings from .tagged import tag_object @@ -175,8 +176,11 @@ def recurse_with_json_ids(tree, json_id): try: result = tree.__class__(contents) except TypeError: - # the derived class' signature is different - # erase the type + # The derived class signature is different, so simply store the + # list representing the contents. Currently this is primarly + # intended to handle namedtuple and NamedTuple instances. + msg = "Failed to serialize instance of {}, converting to list instead" + warnings.warn(msg.format(type(tree))) result = contents seen.remove(id_tree) if hasattr(tree, '_tag'): From 47981ad0270f8defe6830c91e19b30dd7bb66df1 Mon Sep 17 00:00:00 2001 From: Daniel D'Avella Date: Wed, 29 Aug 2018 09:52:51 -0400 Subject: [PATCH 3/5] Add option to ignore implicit conversion warnings --- asdf/asdf.py | 22 +++++++++++++++++----- asdf/reference.py | 5 +++-- asdf/treeutil.py | 14 +++++++++++--- 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/asdf/asdf.py b/asdf/asdf.py index 3dabcb380..8b2ed7864 100644 --- a/asdf/asdf.py +++ b/asdf/asdf.py @@ -49,8 +49,9 @@ class AsdfFile(versioning.VersionedMixin): The main class that represents an ASDF file object. """ def __init__(self, tree=None, uri=None, extensions=None, version=None, - ignore_version_mismatch=True, ignore_unrecognized_tag=False, - copy_arrays=False, custom_schema=None): + ignore_version_mismatch=True, ignore_unrecognized_tag=False, + ignore_implicit_conversion=False, copy_arrays=False, + custom_schema=None): """ Parameters ---------- @@ -81,6 +82,12 @@ def __init__(self, tree=None, uri=None, extensions=None, version=None, When `True`, do not raise warnings for unrecognized tags. Set to `False` by default. + ignore_implicit_conversion : bool + 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 + as-is. + copy_arrays : bool, optional When `False`, when reading files, attempt to memmap underlying data arrays when possible. @@ -90,6 +97,7 @@ def __init__(self, tree=None, uri=None, extensions=None, version=None, validation pass. This can be used to ensure that particular ASDF files follow custom conventions beyond those enforced by the standard. + """ if custom_schema is not None: @@ -104,6 +112,7 @@ def __init__(self, tree=None, uri=None, extensions=None, version=None, self._process_extensions(extensions) self._ignore_version_mismatch = ignore_version_mismatch self._ignore_unrecognized_tag = ignore_unrecognized_tag + self._ignore_implicit_conversion = ignore_implicit_conversion self._file_format_version = None @@ -1068,9 +1077,12 @@ def find_references(self): Finds all external "JSON References" in the tree and converts them to `reference.Reference` objects. """ - # Set directly to self._tree, since it doesn't need to be - # re-validated. - self._tree = reference.find_references(self._tree, self) + # Since this is the first place that the tree is processed when + # creating a new ASDF object, this is where we pass the option to + # ignore warnings about implicit type conversions. + # Set directly to self._tree, since it doesn't need to be re-validated. + self._tree = reference.find_references(self._tree, self, + ignore_implicit_conversion=self._ignore_implicit_conversion) def resolve_references(self, do_not_fill_defaults=False): """ diff --git a/asdf/reference.py b/asdf/reference.py index d276d9cca..c3ecf4dd7 100644 --- a/asdf/reference.py +++ b/asdf/reference.py @@ -128,7 +128,7 @@ def validate(self, data): pass -def find_references(tree, ctx): +def find_references(tree, ctx, ignore_implicit_conversion=False): """ Find all of the JSON references in the tree, and convert them into `Reference` objects. @@ -138,7 +138,8 @@ def do_find(tree, json_id): return Reference(tree['$ref'], json_id, asdffile=ctx) return tree - return treeutil.walk_and_modify(tree, do_find) + return treeutil.walk_and_modify( + tree, do_find, ignore_implicit_conversion=ignore_implicit_conversion) def resolve_references(tree, ctx, do_not_fill_defaults=False): diff --git a/asdf/treeutil.py b/asdf/treeutil.py index df47d9fba..6bf5c0143 100644 --- a/asdf/treeutil.py +++ b/asdf/treeutil.py @@ -81,7 +81,7 @@ def recurse(tree): return recurse(top) -def walk_and_modify(top, callback): +def walk_and_modify(top, callback, ignore_implicit_conversion=False): """Modify a tree by walking it with a callback function. It also has the effect of doing a deep copy. @@ -106,6 +106,13 @@ def walk_and_modify(top, callback): The callback is called on an instance after all of its children have been visited (depth-first order). + ignore_implicit_conversion : bool + 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`. + + Defaults to `False`. + Returns ------- tree : object @@ -179,8 +186,9 @@ def recurse_with_json_ids(tree, json_id): # The derived class signature is different, so simply store the # list representing the contents. Currently this is primarly # intended to handle namedtuple and NamedTuple instances. - msg = "Failed to serialize instance of {}, converting to list instead" - warnings.warn(msg.format(type(tree))) + if not ignore_implicit_conversion: + msg = "Failed to serialize instance of {}, converting to list instead" + warnings.warn(msg.format(type(tree))) result = contents seen.remove(id_tree) if hasattr(tree, '_tag'): From dabbdc08ce4926103de46d1c54995bfbf5327848 Mon Sep 17 00:00:00 2001 From: Daniel D'Avella Date: Wed, 29 Aug 2018 09:53:03 -0400 Subject: [PATCH 4/5] Add test for implicit conversion warnings. Ignore in other tests --- asdf/tests/helpers.py | 13 +++++++------ asdf/tests/test_yaml.py | 30 +++++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/asdf/tests/helpers.py b/asdf/tests/helpers.py index 38aa05a3b..665c4148d 100644 --- a/asdf/tests/helpers.py +++ b/asdf/tests/helpers.py @@ -144,7 +144,8 @@ def recurse(old, new): def assert_roundtrip_tree(tree, tmpdir, *, asdf_check_func=None, - raw_yaml_check_func=None, write_options={}, extensions=None, + raw_yaml_check_func=None, write_options={}, + init_options={}, extensions=None, tree_match_func='assert_equal'): """ Assert that a given tree saves to ASDF and, when loaded back, @@ -171,7 +172,7 @@ def assert_roundtrip_tree(tree, tmpdir, *, asdf_check_func=None, # First, test writing/reading a BytesIO buffer buff = io.BytesIO() - AsdfFile(tree, extensions=extensions).write_to(buff, **write_options) + AsdfFile(tree, extensions=extensions, **init_options).write_to(buff, **write_options) assert not buff.closed buff.seek(0) with AsdfFile.open(buff, mode='rw', extensions=extensions) as ff: @@ -184,7 +185,7 @@ def assert_roundtrip_tree(tree, tmpdir, *, asdf_check_func=None, asdf_check_func(ff) buff.seek(0) - ff = AsdfFile(extensions=extensions) + ff = AsdfFile(extensions=extensions, **init_options) content = AsdfFile._open_impl(ff, buff, _get_yaml_content=True) buff.close() # We *never* want to get any raw python objects out @@ -195,7 +196,7 @@ def assert_roundtrip_tree(tree, tmpdir, *, asdf_check_func=None, raw_yaml_check_func(content) # Then, test writing/reading to a real file - ff = AsdfFile(tree, extensions=extensions) + ff = AsdfFile(tree, extensions=extensions, **init_options) ff.write_to(fname, **write_options) with AsdfFile.open(fname, mode='rw', extensions=extensions) as ff: assert_tree_match(tree, ff.tree, ff, funcname=tree_match_func) @@ -205,7 +206,7 @@ def assert_roundtrip_tree(tree, tmpdir, *, asdf_check_func=None, # Make sure everything works without a block index write_options['include_block_index'] = False buff = io.BytesIO() - AsdfFile(tree, extensions=extensions).write_to(buff, **write_options) + AsdfFile(tree, extensions=extensions, **init_options).write_to(buff, **write_options) assert not buff.closed buff.seek(0) with AsdfFile.open(buff, mode='rw', extensions=extensions) as ff: @@ -219,7 +220,7 @@ def assert_roundtrip_tree(tree, tmpdir, *, asdf_check_func=None, if not INTERNET_OFF and not sys.platform.startswith('win'): server = RangeHTTPServer() try: - ff = AsdfFile(tree, extensions=extensions) + ff = AsdfFile(tree, extensions=extensions, **init_options) ff.write_to(os.path.join(server.tmpdir, 'test.asdf'), **write_options) with AsdfFile.open(server.url + 'test.asdf', mode='r', extensions=extensions) as ff: diff --git a/asdf/tests/test_yaml.py b/asdf/tests/test_yaml.py index 3e739f7fc..8ce8d9158 100644 --- a/asdf/tests/test_yaml.py +++ b/asdf/tests/test_yaml.py @@ -96,8 +96,12 @@ def check_asdf(asdf): def check_raw_yaml(content): assert b'tuple' not in content + # Ignore these warnings for the tests that don't actually test the warning + init_options = dict(ignore_implicit_conversion=True) + helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf, - raw_yaml_check_func=check_raw_yaml) + raw_yaml_check_func=check_raw_yaml, + init_options=init_options) def test_python_tuple(tmpdir): @@ -145,7 +149,9 @@ def test_named_tuple_collections_recursive(tmpdir): def check_asdf(asdf): assert (asdf.tree['val'][2] == np.ones(3)).all() - helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf) + init_options = dict(ignore_implicit_conversion=True) + helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf, + init_options=init_options) def test_named_tuple_typing_recursive(tmpdir): @@ -159,7 +165,25 @@ def test_named_tuple_typing_recursive(tmpdir): def check_asdf(asdf): assert (asdf.tree['val'][2] == np.ones(3)).all() - helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf) + init_options = dict(ignore_implicit_conversion=True) + helpers.assert_roundtrip_tree(tree, tmpdir, asdf_check_func=check_asdf, + init_options=init_options) + + +def test_implicit_conversion_warning(): + nt = namedtuple("TestTupleWarning", ("one", "two", "three")) + + tree = { + "val": nt(1, 2, np.ones(3)) + } + + with pytest.warns(UserWarning, match="Failed to serialize instance"): + with asdf.AsdfFile(tree) as af: + pass + + with pytest.warns(None) as w: + with asdf.AsdfFile(tree, ignore_implicit_conversion=True) as af: + assert len(w) == 0 def test_tags_removed_after_load(tmpdir): From 975503f5979e57d259ced5d0c255089aa8195566 Mon Sep 17 00:00:00 2001 From: Daniel D'Avella Date: Wed, 29 Aug 2018 09:53:50 -0400 Subject: [PATCH 5/5] Update change log --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index e90ba3b94..e5bcfedb4 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,8 @@ - Store ASDF-in-FITS data inside a 1x1 BINTABLE HDU. [#519] +- Allow implicit conversion of ``namedtuple`` into serializable types. [#534] + 2.0.3 (unreleased) ------------------