Skip to content

Propagate NaN star coordinates through apparent positions - #1156

Open
cstolting-collab wants to merge 1 commit into
skyfielders:masterfrom
cstolting-collab:fix/nan-star-apparent-1128
Open

cstolting-collab wants to merge 1 commit into
skyfielders:masterfrom
cstolting-collab:fix/nan-star-apparent-1128

Conversation

@cstolting-collab

Copy link
Copy Markdown

Calling .apparent() on a star with missing coordinates currently sends a NaN date to jplephem and raises a misleading EphemerisRangeError. In a catalog array, this also prevents valid stars from returning positions. This change keeps the unknown positions as NaN while valid rows continue normally.

In _compute_deflector_position(), use the observation time when the clipped closest-approach offset is NaN. The target coordinates remain untouched and propagate NaN through the deflection and aberration calculations. Finite offsets follow the same calculation as before, and real out-of-range deflector dates still raise.

The first Hipparcos example now explicitly drops rows without RA/Dec. Star.from_dataframe() continues to preserve rows and order: filtering inside that constructor would otherwise leave output coordinates misaligned with the caller's original dataframe (including the guide's magnitude-based plotting example). The guide explains how to keep unknown rows.

Validation on Linux, Python 3.12.14, NumPy 2.3.5, pandas 2.2.3, jplephem 2.24:

  • Four new regression tests fail on unchanged upstream and pass with this change. They cover scalar NaN fields, scalar and array times, geocentric and surface observers, mixed/all-NaN/empty arrays, and the bundled Hipparcos excerpt. A fifth test confirms real deflector-range errors still raise.
  • Full Hipparcos catalog: all 118,218 rows complete; the 263 missing-position stars remain NaN. All 117,955 valid XYZ positions are byte-identical to filtered, unmodified upstream output for both observers. Each of the 263 incomplete stars was also checked individually for both observers, with warnings and floating-point errors raised.
  • test-code.sh: upstream has 1 failure out of 584 tests; this branch has the same failure out of 589 tests (test_building_time_from_list_of_utc_datetimes, an unrelated integer/float casting error). Pyflakes passes.
  • The Stars page's 11 Sphinx doctests pass with the bundled data (Sphinx 9.1.1, isolated page build).

Catalog source: https://vizier.cfa.harvard.edu/ftp/cats/I/239/hip_main.dat

SHA-256: 58ceabb104d647160d9437ce6e513a02a036bb4ad9f8879a5a22fd52943616e0

Addresses the NaN-star part of #1128 and follows Brandon's preference in #1128 (comment). This builds on the diagnosis by ajorennie and karimosama7. The separate rise/set issue remains covered by #1145.

Reproduction scripts, catalog evidence, and the separately scoped almanac follow-up are available in the published review package. The CML review harness is optional and adds no dependency to Skyfield.

Prepared and tested with Codex assistance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant