Skip to content

Commit

Permalink
Don't issue warnings for array-valued MIN and MAX
Browse files Browse the repository at this point in the history
  • Loading branch information
tomdonaldson committed Oct 21, 2024
1 parent 009a9e5 commit 6716f67
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 8 deletions.
108 changes: 108 additions & 0 deletions astropy/io/votable/tests/test_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import io
from contextlib import nullcontext

import numpy as np
import pytest

from astropy.io.votable import tree
Expand Down Expand Up @@ -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 = """<VOTABLE xmlns="http://www.ivoa.net/xml/VOTable/v1.3" version="1.5">
<RESOURCE type="results">
<TABLE ID="result" name="result">
<{param_or_field} arraysize="2" datatype="{dt}" name="param_or_field" xtype="interval">
<VALUES><MIN value="{min}"/><MAX value="{max}"/></VALUES>
</{param_or_field}>
</TABLE>
</RESOURCE>
</VOTABLE>
"""

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'.
Expand Down
39 changes: 31 additions & 8 deletions astropy/io/votable/tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 6716f67

Please sign in to comment.