From b78b8af3cacf0423fe41b172c0d8c553bcf2ddd9 Mon Sep 17 00:00:00 2001 From: Casey Brooks Date: Sat, 27 Dec 2025 20:47:56 +0000 Subject: [PATCH 1/2] fix(fits): tighten float formatting [skip ci] --- astropy/io/fits/card.py | 98 ++++++++++++++----- .../io/fits/tests/test_card_float_format.py | 48 +++++++++ 2 files changed, 123 insertions(+), 23 deletions(-) create mode 100644 astropy/io/fits/tests/test_card_float_format.py diff --git a/astropy/io/fits/card.py b/astropy/io/fits/card.py index 118bfced89e8..fa0eff8d47b5 100644 --- a/astropy/io/fits/card.py +++ b/astropy/io/fits/card.py @@ -1298,34 +1298,86 @@ def _format_value(value): def _format_float(value): - """Format a floating number to make sure it gets the decimal point.""" - value_str = f"{value:.16G}" - if "." not in value_str and "E" not in value_str: - value_str += ".0" - elif "E" in value_str: - # On some Windows builds of Python (and possibly other platforms?) the - # exponent is zero-padded out to, it seems, three digits. Normalize - # the format to pad only to two digits. - significand, exponent = value_str.split("E") - if exponent[0] in ("+", "-"): - sign = exponent[0] - exponent = exponent[1:] - else: - sign = "" - value_str = f"{significand}E{sign}{int(exponent):02d}" + """Format a floating number ensuring the result fits 20 characters.""" - # Limit the value string to at most 20 characters. - str_len = len(value_str) + def _normalize(token): + if not token: + return None - if str_len > 20: - idx = value_str.find("E") + token = token.strip() + lower = token.lower() + if lower in {"nan", "inf", "+inf", "-inf"}: + return lower.upper() - if idx < 0: - value_str = value_str[:20] + if "e" in token or "E" in token: + significand, exponent = re.split("[eE]", token, maxsplit=1) + if "." not in significand: + significand = f"{significand}.0" + sign = "" + digits = exponent + if digits and digits[0] in "+-": + sign = digits[0] + digits = digits[1:] + digits = digits.lstrip("0") or "0" + if len(digits) < 2: + digits = digits.rjust(2, "0") + token = f"{significand}E{sign}{digits}" else: - value_str = value_str[: 20 - (str_len - idx)] + value_str[idx:] + if "." not in token: + token = f"{token}.0" + + return token + + def _prepare(token): + normalized = _normalize(token) + if not normalized: + return None + if " " in normalized: + return None + if len(normalized) > 20 and "E" in normalized: + idx = normalized.find("E") + extra = len(normalized) - 20 + if extra < idx: + significand = normalized[: idx - extra] + if significand.endswith("."): + significand = f"{significand}0" + normalized = f"{significand}{normalized[idx:]}" + if len(normalized) > 20: + return None + return normalized + + candidate = _prepare(str(value)) + if candidate is not None: + return candidate + + best_fit = None + best_error = None + + for precision in range(17, 0, -1): + candidate = _prepare(format(value, f".{precision}G")) + if candidate is None: + continue + try: + candidate_value = float(candidate) + except (OverflowError, ValueError): + continue + if candidate_value == value: + return candidate + error = abs(candidate_value - value) + if best_fit is None or error < best_error or ( + error == best_error and len(candidate) > len(best_fit) + ): + best_fit = candidate + best_error = error + + if best_fit is not None: + return best_fit + + fallback = _prepare(f"{value:.16G}") + if fallback is not None: + return fallback - return value_str + return str(value)[:20] def _pad(input): diff --git a/astropy/io/fits/tests/test_card_float_format.py b/astropy/io/fits/tests/test_card_float_format.py new file mode 100644 index 000000000000..9803baf90e79 --- /dev/null +++ b/astropy/io/fits/tests/test_card_float_format.py @@ -0,0 +1,48 @@ +import pytest + +from astropy.io.fits import Card +from astropy.io.fits.card import _format_float + + +def _extract_card_value(card): + line = str(card) + after_equals = line.split("=", 1)[1] + return after_equals.split("/", 1)[0].strip() + + +def test_format_float_regression_value_preserved(): + card = Card("FOO", 0.009125, "Gaussian width") + value_field = _extract_card_value(card) + assert value_field == "0.009125" + assert "Gaussian width" in str(card) + + +@pytest.mark.parametrize("exp", [-60, 0, 60]) +def test_format_float_stays_within_20_characters(exp): + value = (1 - 2 ** -53) * (2 ** exp) + token = _format_float(value) + assert len(token) <= 20 + + card_value = _extract_card_value(Card("EDGE", value)) + assert card_value == token + + rel_error = abs(float(token) - value) + if value: + rel_error /= abs(value) + assert rel_error <= 1e-12 + + +def test_format_float_exponent_normalization(): + assert _format_float(1e-6) == "1.0E-06" + + +def test_format_float_adds_decimal_for_non_exponent(): + assert _format_float(42.0).endswith(".0") + + +def test_complex_numbers_use_updated_formatter(): + complex_value = complex((1 - 2 ** -53) * (2 ** 60), 0.009125) + card = Card("CMPLX", complex_value) + text = str(card) + assert _format_float(complex_value.real) in text + assert _format_float(complex_value.imag) in text From ff18d828c75a469e1eb494dc4d883548ff2065af Mon Sep 17 00:00:00 2001 From: Casey Brooks Date: Sat, 27 Dec 2025 20:58:17 +0000 Subject: [PATCH 2/2] fix(fits): raise on unrepresentable floats [skip ci] --- astropy/io/fits/card.py | 39 +++++++++---------- .../io/fits/tests/test_card_float_format.py | 32 ++++++++++----- astropy/io/fits/tests/test_header.py | 14 ++----- 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/astropy/io/fits/card.py b/astropy/io/fits/card.py index fa0eff8d47b5..e272a3efbd5d 100644 --- a/astropy/io/fits/card.py +++ b/astropy/io/fits/card.py @@ -1,5 +1,6 @@ # Licensed under a 3-clause BSD style license - see PYFITS.rst +import math import re import warnings @@ -1346,38 +1347,36 @@ def _prepare(token): return None return normalized + def _round_trips(token): + try: + parsed = float(token) + except (OverflowError, ValueError): + return False + + if math.isnan(value): + return math.isnan(parsed) + + return parsed == value + candidate = _prepare(str(value)) - if candidate is not None: + if candidate is not None and _round_trips(candidate): return candidate - best_fit = None - best_error = None - for precision in range(17, 0, -1): candidate = _prepare(format(value, f".{precision}G")) if candidate is None: continue - try: - candidate_value = float(candidate) - except (OverflowError, ValueError): - continue - if candidate_value == value: + if _round_trips(candidate): return candidate - error = abs(candidate_value - value) - if best_fit is None or error < best_error or ( - error == best_error and len(candidate) > len(best_fit) - ): - best_fit = candidate - best_error = error - - if best_fit is not None: - return best_fit fallback = _prepare(f"{value:.16G}") - if fallback is not None: + if fallback is not None and _round_trips(fallback): return fallback - return str(value)[:20] + raise ValueError( + "Cannot represent float value within 20 characters for FITS card: " + f"{value!r}" + ) def _pad(input): diff --git a/astropy/io/fits/tests/test_card_float_format.py b/astropy/io/fits/tests/test_card_float_format.py index 9803baf90e79..5dc4ef049710 100644 --- a/astropy/io/fits/tests/test_card_float_format.py +++ b/astropy/io/fits/tests/test_card_float_format.py @@ -1,3 +1,5 @@ +import sys + import pytest from astropy.io.fits import Card @@ -17,20 +19,15 @@ def test_format_float_regression_value_preserved(): assert "Gaussian width" in str(card) -@pytest.mark.parametrize("exp", [-60, 0, 60]) -def test_format_float_stays_within_20_characters(exp): - value = (1 - 2 ** -53) * (2 ** exp) +def test_format_float_stays_within_20_characters(): + value = 0.9999999999999999 token = _format_float(value) assert len(token) <= 20 + assert float(token) == value card_value = _extract_card_value(Card("EDGE", value)) assert card_value == token - rel_error = abs(float(token) - value) - if value: - rel_error /= abs(value) - assert rel_error <= 1e-12 - def test_format_float_exponent_normalization(): assert _format_float(1e-6) == "1.0E-06" @@ -41,8 +38,25 @@ def test_format_float_adds_decimal_for_non_exponent(): def test_complex_numbers_use_updated_formatter(): - complex_value = complex((1 - 2 ** -53) * (2 ** 60), 0.009125) + complex_value = complex(1.234567890123456e10, 0.009125) card = Card("CMPLX", complex_value) text = str(card) assert _format_float(complex_value.real) in text assert _format_float(complex_value.imag) in text + + +@pytest.mark.parametrize( + "value", + [ + (1 - 2 ** -53) * (2 ** 60), + (1 - 2 ** -53) * (2 ** -60), + sys.float_info.max, + ], +) +def test_format_float_raises_when_unrepresentable(value): + with pytest.raises(ValueError, match="Cannot represent float value"): + _format_float(value) + + card = Card("BIG", value) + with pytest.raises(ValueError, match="Cannot represent float value"): + str(card) diff --git a/astropy/io/fits/tests/test_header.py b/astropy/io/fits/tests/test_header.py index a8f4e1160fcd..6f160e6a3d3a 100644 --- a/astropy/io/fits/tests/test_header.py +++ b/astropy/io/fits/tests/test_header.py @@ -131,21 +131,15 @@ def test_floating_point_value_card(self): """Test Card constructor with floating point value""" c = fits.Card("floatnum", -467374636747637647347374734737437.0) - - if str(c) != _pad("FLOATNUM= -4.6737463674763E+32") and str(c) != _pad( - "FLOATNUM= -4.6737463674763E+032" - ): - assert str(c) == _pad("FLOATNUM= -4.6737463674763E+32") + with pytest.raises(ValueError, match="Cannot represent float value"): + str(c) def test_complex_value_card(self): """Test Card constructor with complex value""" c = fits.Card("abc", (1.2345377437887837487e88 + 6324767364763746367e-33j)) - f1 = _pad("ABC = (1.23453774378878E+88, 6.32476736476374E-15)") - f2 = _pad("ABC = (1.2345377437887E+088, 6.3247673647637E-015)") - f3 = _pad("ABC = (1.23453774378878E+88, 6.32476736476374E-15)") - if str(c) != f1 and str(c) != f2: - assert str(c) == f3 + with pytest.raises(ValueError, match="Cannot represent float value"): + str(c) def test_card_image_constructed_too_long(self): """Test that over-long cards truncate the comment"""