From 79ffb1fa0cad00f29fda9963b8bfdf41d6fe33a0 Mon Sep 17 00:00:00 2001 From: zacharyburnett Date: Wed, 17 Jul 2024 14:25:57 -0400 Subject: [PATCH 1/7] handle case where masked array has no true --- asdf/_core/_converters/ndarray.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asdf/_core/_converters/ndarray.py b/asdf/_core/_converters/ndarray.py index d5b312f3e..5a59b9c80 100644 --- a/asdf/_core/_converters/ndarray.py +++ b/asdf/_core/_converters/ndarray.py @@ -125,7 +125,7 @@ def to_yaml_tree(self, obj, tag, ctx): if strides is not None: result["strides"] = list(strides) - if isinstance(data, ma.MaskedArray) and np.any(data.mask): + if isinstance(data, ma.MaskedArray): if options.storage_type == "inline": ctx._blocks._set_array_storage(data.mask, "inline") From ab607727d6e68df2d45ab379804f33a6bb394ae4 Mon Sep 17 00:00:00 2001 From: zacharyburnett Date: Wed, 17 Jul 2024 14:48:58 -0400 Subject: [PATCH 2/7] add unit test for masked array roundtripping --- asdf/_tests/test_masked.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 asdf/_tests/test_masked.py diff --git a/asdf/_tests/test_masked.py b/asdf/_tests/test_masked.py new file mode 100644 index 000000000..b126cf5ae --- /dev/null +++ b/asdf/_tests/test_masked.py @@ -0,0 +1,26 @@ +import numpy.ma +import pytest + +import asdf +from asdf._tests._helpers import assert_tree_match + + +@pytest.mark.parametrize( + "mask", + [ + [[False, False, True], [False, True, False], [False, False, False]], + True, + False, + ], +) +def test_masked(mask, tmp_path): + tree = { + "a": 1, + "b": numpy.ma.array([[1, 0, 0], [0, 1, 0], [0, 0, 0]], mask=mask), + } + af = asdf.AsdfFile(tree) + fn = tmp_path / "masked.asdf" + af.write_to(fn) + + with asdf.open(fn) as af: + assert_tree_match(tree, af.tree) From 15d3c3aa02074d921b2041ec2385e8b353fe4a51 Mon Sep 17 00:00:00 2001 From: zacharyburnett Date: Thu, 18 Jul 2024 09:09:17 -0400 Subject: [PATCH 3/7] move test to test_ndarray --- asdf/_tests/tags/core/tests/test_ndarray.py | 25 +++++++++++++------- asdf/_tests/test_masked.py | 26 --------------------- 2 files changed, 16 insertions(+), 35 deletions(-) delete mode 100644 asdf/_tests/test_masked.py diff --git a/asdf/_tests/tags/core/tests/test_ndarray.py b/asdf/_tests/tags/core/tests/test_ndarray.py index ea830fca5..4ab62299a 100644 --- a/asdf/_tests/tags/core/tests/test_ndarray.py +++ b/asdf/_tests/tags/core/tests/test_ndarray.py @@ -11,6 +11,7 @@ from numpy.testing import assert_array_equal import asdf +from asdf._tests._helpers import assert_tree_match from asdf.exceptions import ValidationError from asdf.extension import Converter, Extension, TagDefinition from asdf.tags.core import ndarray @@ -378,17 +379,23 @@ def test_inline_bare(): assert_array_equal(ff.tree["arr"], [[1, 2, 3, 4], [5, 6, 7, 8]]) -def test_mask_roundtrip(tmp_path): - x = np.arange(0, 10, dtype=float) - m = ma.array(x, mask=x > 5) - tree = {"masked_array": m, "unmasked_array": x} +@pytest.mark.parametrize( + "mask", + [ + [[False, False, True], [False, True, False], [False, False, False]], + True, + False, + ], +) +def test_mask_roundtrip(mask, tmp_path): + array = np.array([[1, 0, 0], [0, 1, 0], [0, 0, 0]]) + tree = { + "unmasked": array, + "masked": np.ma.array(array, mask=mask), + } with roundtrip(tree) as af: - tree = af.tree - - m = tree["masked_array"] - - assert np.all(m.mask[6:]) + assert_tree_match(tree, af.tree) assert len(af._blocks.blocks) == 2 diff --git a/asdf/_tests/test_masked.py b/asdf/_tests/test_masked.py deleted file mode 100644 index b126cf5ae..000000000 --- a/asdf/_tests/test_masked.py +++ /dev/null @@ -1,26 +0,0 @@ -import numpy.ma -import pytest - -import asdf -from asdf._tests._helpers import assert_tree_match - - -@pytest.mark.parametrize( - "mask", - [ - [[False, False, True], [False, True, False], [False, False, False]], - True, - False, - ], -) -def test_masked(mask, tmp_path): - tree = { - "a": 1, - "b": numpy.ma.array([[1, 0, 0], [0, 1, 0], [0, 0, 0]], mask=mask), - } - af = asdf.AsdfFile(tree) - fn = tmp_path / "masked.asdf" - af.write_to(fn) - - with asdf.open(fn) as af: - assert_tree_match(tree, af.tree) From d693b19b989a0359fcf5f38722bb4d8540e5943a Mon Sep 17 00:00:00 2001 From: zacharyburnett Date: Thu, 18 Jul 2024 09:12:53 -0400 Subject: [PATCH 4/7] add change log entry --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3a625fe33..acf53aaf7 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,7 @@ 3.4.0 (unreleased) ------------------ -- +- Fix issue where roundtripping a masked array with no masked values removes the mask [#1803] 3.3.0 (2024-07-12) ------------------ From 503595c362a988d021b2c9f76b458ddeaa0c2fa1 Mon Sep 17 00:00:00 2001 From: zacharyburnett Date: Thu, 18 Jul 2024 09:48:59 -0400 Subject: [PATCH 5/7] numpy assertion helpers ignore masks --- asdf/_tests/tags/core/tests/test_ndarray.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/asdf/_tests/tags/core/tests/test_ndarray.py b/asdf/_tests/tags/core/tests/test_ndarray.py index 4ab62299a..226b34702 100644 --- a/asdf/_tests/tags/core/tests/test_ndarray.py +++ b/asdf/_tests/tags/core/tests/test_ndarray.py @@ -11,7 +11,6 @@ from numpy.testing import assert_array_equal import asdf -from asdf._tests._helpers import assert_tree_match from asdf.exceptions import ValidationError from asdf.extension import Converter, Extension, TagDefinition from asdf.tags.core import ndarray @@ -395,7 +394,10 @@ def test_mask_roundtrip(mask, tmp_path): } with roundtrip(tree) as af: - assert_tree_match(tree, af.tree) + # assert_array_equal ignores the mask, so use equality here + assert (tree["masked"] == af["masked"]).all() + # ensure tree validity + assert af["unmasked"] == af["masked"].data assert len(af._blocks.blocks) == 2 @@ -527,7 +529,8 @@ def test_inline_masked_array(tmp_path): with asdf.open(testfile) as f2: assert len(list(f2._blocks.blocks)) == 0 - assert_array_equal(f.tree["test"], f2.tree["test"]) + # assert_array_equal ignores the mask, so use equality here + assert (f.tree["test"] == f2.tree["test"]).all() with open(testfile, "rb") as fd: assert b"null" in fd.read() From e51a369905094ed264f208b0caf409c12774712d Mon Sep 17 00:00:00 2001 From: Zach Burnett Date: Thu, 18 Jul 2024 11:56:15 -0400 Subject: [PATCH 6/7] use `.all()` Co-authored-by: Brett Graham --- asdf/_tests/tags/core/tests/test_ndarray.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/asdf/_tests/tags/core/tests/test_ndarray.py b/asdf/_tests/tags/core/tests/test_ndarray.py index 226b34702..031dd4948 100644 --- a/asdf/_tests/tags/core/tests/test_ndarray.py +++ b/asdf/_tests/tags/core/tests/test_ndarray.py @@ -397,7 +397,7 @@ def test_mask_roundtrip(mask, tmp_path): # assert_array_equal ignores the mask, so use equality here assert (tree["masked"] == af["masked"]).all() # ensure tree validity - assert af["unmasked"] == af["masked"].data + assert (af["unmasked"] == af["unmasked"]).all() assert len(af._blocks.blocks) == 2 From d0c0b41677a90e585f0ffa02e679c77d06167c3c Mon Sep 17 00:00:00 2001 From: zacharyburnett Date: Thu, 18 Jul 2024 15:01:08 -0400 Subject: [PATCH 7/7] compare data AND mask --- asdf/_tests/tags/core/tests/test_ndarray.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/asdf/_tests/tags/core/tests/test_ndarray.py b/asdf/_tests/tags/core/tests/test_ndarray.py index 031dd4948..2661cb3a5 100644 --- a/asdf/_tests/tags/core/tests/test_ndarray.py +++ b/asdf/_tests/tags/core/tests/test_ndarray.py @@ -395,9 +395,10 @@ def test_mask_roundtrip(mask, tmp_path): with roundtrip(tree) as af: # assert_array_equal ignores the mask, so use equality here - assert (tree["masked"] == af["masked"]).all() + assert (tree["masked"].data == af["masked"].data).all() + assert (tree["masked"].mask == af["masked"].mask).all() # ensure tree validity - assert (af["unmasked"] == af["unmasked"]).all() + assert (af["unmasked"] == af["masked"].data).all() assert len(af._blocks.blocks) == 2