Skip to content

Preserve observation context when indexing positions - #1151

Open
antonsoo wants to merge 2 commits into
skyfielders:masterfrom
antonsoo:fix/sliced-observation-context
Open

antonsoo wants to merge 2 commits into
skyfielders:masterfrom
antonsoo:fix/sliced-observation-context

Conversation

@antonsoo

Copy link
Copy Markdown

Selecting from an array of observations before computing apparent places currently drops the observer and crashes:

from skyfield.api import load

eph = load('de421.bsp')
t = load.timescale().utc(2025, 1, [1, 2, 3])
position = eph['earth'].at(t).observe(eph['mars'])
print(position[1].apparent().radec()[0])
# Before: AttributeError: 'NoneType' object has no attribute 'xyz'
# After: 08h 17m 38.49s

Keep the ephemeris, corresponding barycentric observer, Earth-relative observer vector and cached light time when indexing an ICRF position. Array observers are indexed with the observation; a scalar observer shared by a star catalogue remains scalar. Barycentric positions retain their self-reference.

This also restores observing from an indexed barycentric position, phase angles after selection, and Earth-occultation checks on selected apparent positions. Regression tests cover integer, slice, repeated/reordered and boolean selections, including a star catalogue with one observer.

The project's test-code.sh passes with the pinned Python 3.8 dependencies, and test-docs.sh and Pyflakes pass. A separate Python 3.12/NumPy 2.5 run retains the existing upstream failures; the added indexing tests pass. Workloads using the shipped JPL ephemeris agree with separately evaluated scalar observations, and complete unselected outputs remain identical on both profiles.

@brandon-rhodes

Copy link
Copy Markdown
Member

I'm busy next week, but will plan to take a look at this in early November. Thanks for the fact that the PR includes a test, that's very helpful.

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.

2 participants