From f6c0f5f9f6f69edfb60dc28ca23ca47baec25926 Mon Sep 17 00:00:00 2001 From: Rowan Ellis Date: Thu, 23 Oct 2025 19:55:51 +0000 Subject: [PATCH 1/2] io.fits.Card: prefer minimal float string to avoid comment truncation; exponent normalization; tests (refs #30) --- astropy/io/fits/card.py | 107 +++++++++++++++++---- astropy/io/fits/tests/test_float_format.py | 81 ++++++++++++++++ 2 files changed, 171 insertions(+), 17 deletions(-) create mode 100644 astropy/io/fits/tests/test_float_format.py diff --git a/astropy/io/fits/card.py b/astropy/io/fits/card.py index 118bfced89e8..60087486dbc1 100644 --- a/astropy/io/fits/card.py +++ b/astropy/io/fits/card.py @@ -1297,25 +1297,98 @@ def _format_value(value): return "" +def _normalize_float_str(s): + """ + Normalize a float string for FITS: + - Uppercase exponent letter to 'E' + - Ensure exponent includes sign and at least two digits + - Ensure integer-like floats include a decimal point (e.g., '2.0') + """ + # Do not alter special floats + sl = s.lower() + if sl in ("nan", "inf", "-inf"): + return s + + # Normalize exponent + if "e" in s or "E" in s: + s = s.replace("e", "E") + parts = s.split("E", 1) + mant = parts[0] + exp = parts[1] if len(parts) > 1 else "" + # Ensure sign is present + if not exp.startswith(("+", "-")): + exp = "+" + exp + sign = exp[0] + digits = exp[1:] + # Defensive: strip any stray signs inside digits + if digits.startswith(("+", "-")): + digits = digits[1:] + # Pad to at least two digits + if len(digits) < 2: + digits = digits.rjust(2, "0") + s = mant + "E" + sign + digits + # Ensure integer-like floats include a decimal point + if ("E" not in s) and ("." not in s): + s = s + ".0" + return s + 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 float according to FITS card value rules: + - Prefer Python's minimal round-trip representation (str(value)) when it fits + within the 20-character numeric field; otherwise fall back to general + formatting with normalization and trimming. + - Normalize exponent to uppercase 'E' with sign and at least two digits. + - Ensure integer-like floats include a decimal point. + - Special floats (NaN/Inf) behavior unchanged. + """ + # Primary candidate: minimal round-trip representation + try: + s1 = str(value) + except Exception: + s1 = f"{value:.16G}" + sl = s1.lower() + # Preserve special float behavior unchanged + if sl in ("nan", "inf", "-inf"): + return s1 + s1 = _normalize_float_str(s1) + if len(s1) <= 20: + return s1 + + # Fallback to general formatting with normalization + s2 = f"{value:.16G}" + s2 = _normalize_float_str(s2) + if len(s2) <= 20: + return s2 + + # Still too long: trim to <= 20 chars, preserving exponent if present + if "E" in s2: + mant, exp_digits = s2.split("E", 1) + exp = "E" + exp_digits # already normalized to sign+2 digits + avail = 20 - len(exp) + if avail <= 0: + # Last resort: hard truncate to 20 chars + out = (mant + exp)[:20] + # Avoid trailing bare dot + if out.endswith("."): + out = out[:-1] or "0" + return out + mant = mant[:avail] + if mant and mant[-1] == ".": + mant = mant[:-1] or "0" + out = mant + exp + if len(out) > 20: + out = out[:20] + if out.endswith("."): + out = out[:-1] or "0" + return out + + # No exponent and still too long: truncate to 20, avoid trailing '.' + out = s2[:20] + if out.endswith("."): + out = out[:-1] or "0" + return out - # Limit the value string to at most 20 characters. - str_len = len(value_str) if str_len > 20: idx = value_str.find("E") diff --git a/astropy/io/fits/tests/test_float_format.py b/astropy/io/fits/tests/test_float_format.py new file mode 100644 index 000000000000..e91a954522f5 --- /dev/null +++ b/astropy/io/fits/tests/test_float_format.py @@ -0,0 +1,81 @@ +import pytest +from astropy.io import fits +from astropy.io.fits.verify import VerifyWarning + + +def _value_field(card_str): + """ + Extract the fixed-width value field (up to 20 characters) after '='. + For standard cards the field is exactly 20 characters and right-justified. + """ + if '=' not in card_str: + return '' + eq = card_str.index('=') + # After '=' there is typically a space before the field + start = eq + 1 + if start < len(card_str) and card_str[start] == ' ': + start += 1 + end = min(start + 20, len(card_str)) + return card_str[start:end] + + +def test_hierarch_float_minimal_repr_no_truncate(): + c = fits.Card( + 'HIERARCH ESO IFM CL RADIUS', + 0.009125, + '[m] radius arround actuator to avoid' + ) + with pytest.warns(None) as rec: + s = str(c) + # No VerifyWarning + assert not any(isinstance(w.message, VerifyWarning) for w in rec) + # Card is exactly 80 chars + assert len(s) == 80 + # Value uses minimal representation and comment preserved + assert '= 0.009125 /' in s + assert s.endswith('[m] radius arround actuator to avoid') + + +def test_float_minimal_repr_exponent_uppercase(): + c = fits.Card('FOO', 1e-5) + s = str(c) + # Uppercase exponent with two digits + assert 'E-05' in s + # Value field is right-justified within 20 chars for standard card + vf = _value_field(s) + assert len(vf) == 20 + assert vf.endswith('E-05') + # Leading spaces indicate right-justification + assert vf[0] == ' ' + + +def test_float_integer_like_has_decimal_point(): + c = fits.Card('BAR', 2.0) + s = str(c) + vf = _value_field(s) + assert '2.0' in vf + + +def test_complex_minimal_repr(): + c = fits.Card('BAZ', complex(0.009125, -5e-6)) + s = str(c) + # Real part minimal, imag part uses normalized small exponent + assert '0.009125' in s + assert 'E-06' in s + vf = _value_field(s) + # Entire complex literal should fit in the 20-char field in standard cards + assert len(vf) <= 20 + + +def test_hierarch_equal_sign_shortening(): + # Construct a HIERARCH card near the limit to force '='-shortening logic + # The specific keyword and value combo aims to exceed by 1 char before shortening + key = 'HIERARCH ESO VERY LONG KEYWORD TEST' + comment = 'C' * 30 + c = fits.Card(key, 0.009125, comment) + with pytest.warns(None) as rec: + s = str(c) + # Shortening occurs without raising errors; card length should be 80 + assert len(s) == 80 + # No VerifyWarning + assert not any(isinstance(w.message, VerifyWarning) for w in rec) From 680f56ff89f3072863eef674c3751c2a5c49e6cb Mon Sep 17 00:00:00 2001 From: Casey Quinn Date: Thu, 23 Oct 2025 20:09:09 +0000 Subject: [PATCH 2/2] feat(fits-card): normalize exponent formatting and precision-based mantissa rounding; remove dead code; add tests for -0.0 and large exponent; clarify test helper comments --- astropy/io/fits/card.py | 110 +++++++++------------ astropy/io/fits/tests/test_float_format.py | 23 ++++- 2 files changed, 68 insertions(+), 65 deletions(-) diff --git a/astropy/io/fits/card.py b/astropy/io/fits/card.py index 60087486dbc1..c3197b92041d 100644 --- a/astropy/io/fits/card.py +++ b/astropy/io/fits/card.py @@ -1301,8 +1301,10 @@ def _normalize_float_str(s): """ Normalize a float string for FITS: - Uppercase exponent letter to 'E' - - Ensure exponent includes sign and at least two digits - - Ensure integer-like floats include a decimal point (e.g., '2.0') + - Exponent normalization: include a sign and at least two digits only + if abs(exponent) < 100; otherwise preserve full width without extra + padding zeros. + - Ensure integer-like floats include a decimal point (e.g., '2.0'). """ # Do not alter special floats sl = s.lower() @@ -1312,21 +1314,29 @@ def _normalize_float_str(s): # Normalize exponent if "e" in s or "E" in s: s = s.replace("e", "E") - parts = s.split("E", 1) - mant = parts[0] - exp = parts[1] if len(parts) > 1 else "" - # Ensure sign is present - if not exp.startswith(("+", "-")): + mant, exp = s.split("E", 1) + # Remove spaces in exponent part and ensure a sign + exp = exp.replace(" ", "") + if not exp or exp[0] not in "+-": exp = "+" + exp - sign = exp[0] - digits = exp[1:] - # Defensive: strip any stray signs inside digits - if digits.startswith(("+", "-")): - digits = digits[1:] - # Pad to at least two digits - if len(digits) < 2: - digits = digits.rjust(2, "0") - s = mant + "E" + sign + digits + # Parse exponent as integer + try: + exp_int = int(exp) + except ValueError: + # Fallback: strip sign and parse digits only + sign_char = exp[0] if exp and exp[0] in "+-" else "+" + digits = exp[1:] + try: + exp_int = int(("-" if sign_char == "-" else "") + (digits or "0")) + except ValueError: + exp_int = 0 + sign_char = "+" if exp_int >= 0 else "-" + abs_exp = abs(exp_int) + if abs_exp < 100: + digits_out = f"{abs_exp:02d}" + else: + digits_out = str(abs_exp) + s = mant + "E" + sign_char + digits_out # Ensure integer-like floats include a decimal point if ("E" not in s) and ("." not in s): s = s + ".0" @@ -1336,9 +1346,10 @@ def _format_float(value): """ Format a float according to FITS card value rules: - Prefer Python's minimal round-trip representation (str(value)) when it fits - within the 20-character numeric field; otherwise fall back to general - formatting with normalization and trimming. - - Normalize exponent to uppercase 'E' with sign and at least two digits. + within the 20-character numeric field; otherwise use scientific notation + with precision-based rounding. + - Exponent is normalized (uppercase 'E', sign, and at least two digits + only for abs(exp) < 100; otherwise preserve full width). - Ensure integer-like floats include a decimal point. - Special floats (NaN/Inf) behavior unchanged. """ @@ -1351,56 +1362,27 @@ def _format_float(value): # Preserve special float behavior unchanged if sl in ("nan", "inf", "-inf"): return s1 - s1 = _normalize_float_str(s1) - if len(s1) <= 20: - return s1 - - # Fallback to general formatting with normalization - s2 = f"{value:.16G}" - s2 = _normalize_float_str(s2) - if len(s2) <= 20: - return s2 - - # Still too long: trim to <= 20 chars, preserving exponent if present - if "E" in s2: - mant, exp_digits = s2.split("E", 1) - exp = "E" + exp_digits # already normalized to sign+2 digits - avail = 20 - len(exp) - if avail <= 0: - # Last resort: hard truncate to 20 chars - out = (mant + exp)[:20] - # Avoid trailing bare dot - if out.endswith("."): - out = out[:-1] or "0" - return out - mant = mant[:avail] - if mant and mant[-1] == ".": - mant = mant[:-1] or "0" - out = mant + exp - if len(out) > 20: - out = out[:20] - if out.endswith("."): - out = out[:-1] or "0" - return out - - # No exponent and still too long: truncate to 20, avoid trailing '.' - out = s2[:20] + s1n = _normalize_float_str(s1) + if len(s1n) <= 20: + return s1n + + # Use scientific notation with precision-based rounding + # Start with up to 16 digits after decimal and reduce until it fits + N = 16 + while N >= 0: + sci = f"{value:.{N}E}" + sci_n = _normalize_float_str(sci) + if len(sci_n) <= 20: + return sci_n + N -= 1 + + # Fallback: if somehow still too long, take normalized minimal and truncate + out = s1n[:20] if out.endswith("."): out = out[:-1] or "0" return out - if str_len > 20: - idx = value_str.find("E") - - if idx < 0: - value_str = value_str[:20] - else: - value_str = value_str[: 20 - (str_len - idx)] + value_str[idx:] - - return value_str - - def _pad(input): """Pad blank space to the input string to be multiple of 80.""" _len = len(input) diff --git a/astropy/io/fits/tests/test_float_format.py b/astropy/io/fits/tests/test_float_format.py index e91a954522f5..3d9bf23a8fed 100644 --- a/astropy/io/fits/tests/test_float_format.py +++ b/astropy/io/fits/tests/test_float_format.py @@ -6,7 +6,10 @@ def _value_field(card_str): """ Extract the fixed-width value field (up to 20 characters) after '='. - For standard cards the field is exactly 20 characters and right-justified. + For standard FITS cards, the numeric value field is exactly 20 characters + and right-justified after '= '. HIERARCH cards may omit the space or use + a shortened value indicator; this helper is intended for standard cards + in these tests. """ if '=' not in card_str: return '' @@ -79,3 +82,21 @@ def test_hierarch_equal_sign_shortening(): assert len(s) == 80 # No VerifyWarning assert not any(isinstance(w.message, VerifyWarning) for w in rec) + + +def test_negative_zero_preserved(): + c = fits.Card('NEGZERO', -0.0) + s = str(c) + vf = _value_field(s) + # Ensure the sign and decimal point are preserved for -0.0 + assert '-0.0' in vf + + +def test_large_exponent_not_zero_padded(): + c = fits.Card('BIG', 1e100) + s = str(c) + vf = _value_field(s) + # Exponent should be normalized with uppercase E, explicit sign, and + # no forced zero-padding beyond the natural width (E+100 here) + assert 'E+100' in vf + assert len(vf) <= 20