Repository navigation
Propagate NaN star coordinates through apparent positions - #1156
Open
cstolting-collab wants to merge 1 commit into
Open
cstolting-collab wants to merge 1 commit into
cstolting-collab wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Calling
.apparent()on a star with missing coordinates currently sends a NaN date to jplephem and raises a misleadingEphemerisRangeError. 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:
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.Catalog source: https://vizier.cfa.harvard.edu/ftp/cats/I/239/hip_main.dat
SHA-256:
58ceabb104d647160d9437ce6e513a02a036bb4ad9f8879a5a22fd52943616e0Addresses 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.