diff --git a/astropy/io/votable/tests/test_tree.py b/astropy/io/votable/tests/test_tree.py index 3625b216494a..cafd2b786041 100644 --- a/astropy/io/votable/tests/test_tree.py +++ b/astropy/io/votable/tests/test_tree.py @@ -3,6 +3,7 @@ import io from contextlib import nullcontext +import numpy as np import pytest from astropy.io.votable import tree @@ -115,6 +116,113 @@ def test_votable_values_empty_min_max(): parse(io.BytesIO(with_empty_minmax), verify="exception") +def _assert_minmax_match(val, expected_val): + """val can be number or numpy array; expected_val can be number or list""" + if isinstance(val, np.ndarray): + assert np.allclose(val, np.array(expected_val)) + else: + assert val == expected_val + + +@pytest.mark.parametrize( + ("testvals"), + [ + { + "dt": "float", + "min": "1.2", + "max": "3.4", + "expected_min": 1.2, + "expected_max": 3.4, + }, + { + "dt": "float", + "min": "1.2 3.4", + "max": "3.4 5.6", + "expected_min": [1.2, 3.4], + "expected_max": [3.4, 5.6], + }, + { + "dt": "double", + "min": "1.2", + "max": "3.4", + "expected_min": 1.2, + "expected_max": 3.4, + }, + { + "dt": "double", + "min": "1.2 3.4", + "max": "3.4 5.6", + "expected_min": [1.2, 3.4], + "expected_max": [3.4, 5.6], + }, + { + "dt": "unsignedByte", + "min": "1", + "max": "3", + "expected_min": 1, + "expected_max": 3, + }, + { + "dt": "unsignedByte", + "min": "1 3", + "max": "3 5", + "expected_min": [1, 3], + "expected_max": [3, 5], + }, + {"dt": "short", "min": "1", "max": "3", "expected_min": 1, "expected_max": 3}, + { + "dt": "short", + "min": "1 3", + "max": "3 5", + "expected_min": [1, 3], + "expected_max": [3, 5], + }, + {"dt": "int", "min": "1", "max": "3", "expected_min": 1, "expected_max": 3}, + { + "dt": "int", + "min": "1 3", + "max": "3 5", + "expected_min": [1, 3], + "expected_max": [3, 5], + }, + {"dt": "long", "min": "1", "max": "3", "expected_min": 1, "expected_max": 3}, + { + "dt": "long", + "min": "1 3", + "max": "3 5", + "expected_min": [1, 3], + "expected_max": [3, 5], + }, + ], +) +def test_min_max_with_arrays(testvals): + """When a FIELD/PARAM is an array type, ensure we accept (without warnings/exceptions) + MIN and MAX values both as scalars (preferred except for some xtypes) or arrays.""" + minmax_template = """ + + + <{param_or_field} arraysize="2" datatype="{dt}" name="param_or_field" xtype="interval"> + + +
+
+
+ """ + + for param_or_field in ["PARAM", "FIELD"]: + testvals["param_or_field"] = param_or_field + xml_string = minmax_template.format(**testvals) + xml_bytes = xml_string.encode("utf-8") + + # Parse without exceptions. + vot = parse(io.BytesIO(xml_bytes), verify="exception") + + # Assert that the scalar or array values were parsed and stored as expected. + param_or_field = vot.get_field_by_id_or_name("param_or_field") + _assert_minmax_match(param_or_field.values.min, testvals["expected_min"]) + _assert_minmax_match(param_or_field.values.max, testvals["expected_max"]) + + def test_version(): """ VOTableFile.__init__ allows versions of '1.1' through '1.5'. diff --git a/astropy/io/votable/tree.py b/astropy/io/votable/tree.py index 525b2569ca3d..7642aca8a641 100644 --- a/astropy/io/votable/tree.py +++ b/astropy/io/votable/tree.py @@ -1053,10 +1053,7 @@ def min(self): @min.setter def min(self, min): - if hasattr(self._field, "converter") and min is not None: - self._min = self._field.converter.parse(min, config=self._config)[0] - else: - self._min = min + self._min = self._parse_minmax(min) @min.deleter def min(self): @@ -1089,10 +1086,7 @@ def max(self): @max.setter def max(self, max): - if hasattr(self._field, "converter") and max is not None: - self._max = self._field.converter.parse(max, config=self._config)[0] - else: - self._max = max + self._max = self._parse_minmax(max) @max.deleter def max(self): @@ -1166,6 +1160,35 @@ def parse(self, iterator, config): return self + def _parse_minmax(self, val): + retval = val + if hasattr(self._field, "converter") and val is not None: + parsed_val = None + if self._field.arraysize is None: + # Use the default parser. + parsed_val = self._field.converter.parse(val, config=self._config)[0] + else: + # Set config to ignore verification (prevent warnings and exceptions) on parse. + ignore_warning_config = self._config.copy() + ignore_warning_config["verify"] = "ignore" + + # max should be a scalar except for certain xtypes so try scalar parsing first. + try: + parsed_val = self._field.converter.parse_scalar( + val, config=ignore_warning_config + )[0] + except ValueError as ex: + pass # Ignore ValueError returned for array vals by some parsers (like int) + finally: + if parsed_val is None: + # Try the array parsing to support certain xtypes and historical array values. + parsed_val = self._field.converter.parse( + val, config=ignore_warning_config + )[0] + + retval = parsed_val + return retval + def is_defaults(self): """ Are the settings on this ``VALUE`` element all the same as the