Feature: Added JPL Horizons, and TLE Support#134
Feature: Added JPL Horizons, and TLE Support#134SaltyPotatoe wants to merge 2 commits intoppp-one:mainfrom
Conversation
…odule for handling non-sidereal objects, as well as updates to the image handler and observatory modules to support tracking and scheduling for these types of objects. Additionally, new tests have been added to ensure the functionality of the non-sidereal features.
There was a problem hiding this comment.
Pull request overview
This PR extends Astra’s non-sidereal tracking framework to support JPL Horizons resolution (minor bodies) and Earth satellites via TLE strings, including precomputed ephemerides/rates and integration into the observatory imaging loop.
Changes:
- Add TLE + JPL Horizons support to ephemeris/coordinate utilities and object action config resolution.
- Introduce
NonSiderealManagerand integrate pre-pointing, activation timing, rate refresh, and periodic re-centering intoObservatory.image_sequence. - Expand/adjust unit + integration tests (including new network-marked coverage) and update project dependencies/pytest markers.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/astra/utils.py |
Adds Horizons/TLE-aware coordinate resolution + ephemeris precompute + rate derivation helpers. |
src/astra/action_configs.py |
Extends ObjectActionConfig to accept TLE strings and precompute interpolators during visibility validation. |
src/astra/nonsidereal.py |
New manager encapsulating rate application, pre-pointing, activation time, and re-centering behavior. |
src/astra/observatory.py |
Integrates non-sidereal tracking lifecycle into imaging sequences; keeps rates refreshed during long saves. |
src/astra/image_handler.py |
Adjusts observing-night date behavior and strengthens save-time validation. |
src/astra/frontend/file_explorer/file_explorer.py |
Tightens allowed extensions and changes router/base-path injection behavior. |
tests/test_nonsidereal.py |
New unit tests for NonSiderealManager. |
tests/test_observatory_running_schedule.py |
Adds integration scenarios for non-sidereal, Horizons minor bodies, and TLE tracking. |
tests/test_utils.py |
Adds helper tests for ephemeris precompute/rates, plus network-marked Horizons/TLE tests. |
tests/test_visibility_check.py |
Updates mocking approach around lookup-name resolution and non-sidereal setup. |
tests/test_subframe_config.py |
Adds validation tests for the new non-sidereal lead-time field. |
tests/test_image_handler.py |
Updates tests for new ImageHandler behaviors (header validation + observing-night date logic). |
pyproject.toml |
Adds astroquery dependency and a network pytest marker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception as e: | ||
| if not telescope.get("CanSetRightAscensionRate"): | ||
| self.logger.warning( | ||
| "Mount does not support RightAscensionRate " | ||
| "(CanSetRightAscensionRate=False). " | ||
| "Non-sidereal tracking is unavailable." | ||
| ) | ||
| else: | ||
| self.logger.warning(f"Could not set non-sidereal tracking rates: {e}") |
There was a problem hiding this comment.
In the error path, _apply_rates calls telescope.get("CanSetRightAscensionRate") inside the except block. If telescope.set(...) failed due to a connectivity/device error, telescope.get(...) is also likely to raise, which would escape this handler and potentially crash the sequence. Wrap the capability check in its own try/except (or cache capability earlier) so failures remain non-fatal as intended.
|
|
||
|
|
||
| @pytest.mark.slow | ||
| @pytest.mark.network |
There was a problem hiding this comment.
Marking the entire TestScheduleActionTypes class as @pytest.mark.network will also mark tests like open, close, cool_camera, etc. as network-dependent even though they can run purely against local Alpaca simulators. This makes it easy for CI to skip most integration coverage when network tests are deselected. Consider marking only the Horizons/TLE-specific tests as network instead of the whole class.
| @pytest.mark.network |
| duration_hours = (end_time - start_time).to_value("hr") + 0.5 | ||
| try: | ||
| ( | ||
| self._ra_interp, | ||
| self._dec_interp, | ||
| self._ra_rate_interp, | ||
| self._dec_rate_interp, | ||
| ) = precompute_ephemeris( | ||
| self.lookup_name, | ||
| start_time, | ||
| duration_hours, | ||
| observatory_location, | ||
| self.nonsidereal_recenter_interval / 60, | ||
| self.tle, | ||
| return_rates=True, | ||
| ) |
There was a problem hiding this comment.
_resolve_lookup_name passes self.nonsidereal_recenter_interval / 60 directly into precompute_ephemeris. When nonsidereal_recenter_interval is 0 (the default), this becomes interval_minutes=0, which will trigger a division-by-zero in precompute_ephemeris (duration_hours * 60 / interval_minutes). Use a safe default sampling interval when the recenter interval is 0 (e.g., fall back to interval_minutes=1.0), and also validate that nonsidereal_recenter_interval >= 0.
| import logging | ||
| import time | ||
| from datetime import datetime | ||
| from pathlib import Path |
There was a problem hiding this comment.
Path is imported but not used in this module. If ruff/linters are enforced in CI, this will fail lint. Remove the unused import or use it where intended.
| from pathlib import Path |
| observing_date = cls.get_observing_night_date(action.start_time, location) | ||
| location = header.get_observatory_location() | ||
| observing_date = cls.get_observing_night_date( | ||
| datetime.datetime.now(datetime.UTC), location |
There was a problem hiding this comment.
ImageHandler.from_action now computes observing_date using datetime.now() rather than action.start_time. This can put images from a schedule into the wrong observing-night date (e.g., schedules defined in advance, or actions spanning midnight). Use action.start_time as the reference time when deriving observing_date.
| datetime.datetime.now(datetime.UTC), location | |
| action.start_time, location |
| def __repr__(self): | ||
| return ( | ||
| f"ImageHandler(header={dict(self.header)}, " | ||
| f"image_directory={self._image_directory}, " | ||
| f"image_directory={self.image_directory}, " | ||
| f"last_image_path={self.last_image_path}, " | ||
| f"last_image_timestamp={self.last_image_timestamp}, " | ||
| f"filename_templates={self.filename_templates})" |
There was a problem hiding this comment.
__repr__ uses self.image_directory, which raises ValueError when _image_directory is unset. That makes repr(image_handler) unsafe in logs/debugging, especially in error handling paths. Use self._image_directory (or a safe placeholder) instead of the property accessor in __repr__.
| @@ -250,12 +247,6 @@ def create_router( | |||
|
|
|||
| router = APIRouter() | |||
|
|
|||
| files_base = fits_url or "/fits" | |||
| if not files_base.startswith("/"): | |||
| files_base = "/" + files_base | |||
| if not files_base.endswith("/"): | |||
| files_base = files_base + "/" | |||
|
|
|||
| def get_fits_dir() -> Path: | |||
| return fits_dir() if callable(fits_dir) else fits_dir | |||
|
|
|||
| @@ -289,11 +280,7 @@ def root_index(request: Request): | |||
| # Also inject a small inline script that sets window.__ASTRA_FITS_BASE_PATH | |||
| # so the embedded case (fetch + innerHTML) has the correct base for JS | |||
| # even when the browser's window.location.pathname is different. | |||
| injected = ( | |||
| f'<base href="{base}">\n' | |||
| f' <script>window.__ASTRA_FITS_BASE_PATH = "{base}"; ' | |||
| f'window.__ASTRA_FITS_FILES_BASE_PATH = "{files_base}";</script>' | |||
| ) | |||
| injected = f'<base href="{base}">\n <script>window.__ASTRA_FITS_BASE_PATH = "{base}";</script>' | |||
| if "<base" in html: | |||
There was a problem hiding this comment.
create_router no longer takes fits_url, and the injected script no longer sets window.__ASTRA_FITS_FILES_BASE_PATH. The frontend JS (see static/js/viewer/basePath.js) uses that global to determine where to fetch raw FITS files when the host mounts them somewhere other than /fits/. This change will break file downloads/previews for custom fits_url configurations. Reintroduce setting __ASTRA_FITS_FILES_BASE_PATH based on the mounted fits_url (or otherwise make the router aware of it).
| "step": str(n_points - 1), # Horizons returns n+1 rows for n steps | ||
| } | ||
|
|
There was a problem hiding this comment.
epochs['step'] is being set to a bare integer string (str(n_points - 1)). astroquery.jplhorizons.Horizons expects a step size with units (e.g., '1m', '10m', '1h') for start/stop queries; passing a unitless count is not a valid step and can make Horizons queries fail at runtime. Consider using step=f"{interval_minutes}m" (or similar) and drop the n_points→step conversion logic.
| "step": str(n_points - 1), # Horizons returns n+1 rows for n steps | |
| } | |
| "step": f"{minutes:g}m", | |
| } |
| """Persist raw Horizons output locally and mirror the API call input into the logger.""" | ||
| try: | ||
| from astra.config import Config | ||
|
|
||
| horizons_dir = Config().paths.logs / "horizons" | ||
| horizons_dir.mkdir(parents=True, exist_ok=True) | ||
| safe_name = body_name.replace(" ", "_").replace("/", "_") | ||
| output_path = horizons_dir / f"{safe_name}_{context}_{datetime.utcnow().strftime('%Y%m%dT%H%M%S%f')}.ecsv" | ||
| eph.write(output_path, format="ascii.ecsv", overwrite=True) | ||
| logger.warning("Saved raw Horizons output for %s (%s) to %s", body_name, context, output_path) | ||
| except Exception as exc: | ||
| logger.warning("Failed to save raw Horizons output for %s (%s): %s", body_name, context, exc) | ||
|
|
||
| try: | ||
| logger.warning("Horizons API call input for %s (%s): %s", body_name, context, call_input) |
There was a problem hiding this comment.
_save_and_log_horizons_output writes every successful Horizons ephemeris to disk and logs at WARNING level. This will create noisy logs and unbounded disk usage in normal operation (non-error path), especially if schedules are loaded frequently. Suggest gating this behind a debug flag / config option, and lowering the log level (e.g., DEBUG) unless the call is part of an error/diagnostic path.
| """Persist raw Horizons output locally and mirror the API call input into the logger.""" | |
| try: | |
| from astra.config import Config | |
| horizons_dir = Config().paths.logs / "horizons" | |
| horizons_dir.mkdir(parents=True, exist_ok=True) | |
| safe_name = body_name.replace(" ", "_").replace("/", "_") | |
| output_path = horizons_dir / f"{safe_name}_{context}_{datetime.utcnow().strftime('%Y%m%dT%H%M%S%f')}.ecsv" | |
| eph.write(output_path, format="ascii.ecsv", overwrite=True) | |
| logger.warning("Saved raw Horizons output for %s (%s) to %s", body_name, context, output_path) | |
| except Exception as exc: | |
| logger.warning("Failed to save raw Horizons output for %s (%s): %s", body_name, context, exc) | |
| try: | |
| logger.warning("Horizons API call input for %s (%s): %s", body_name, context, call_input) | |
| """Persist raw Horizons diagnostics when explicitly enabled by configuration.""" | |
| try: | |
| from astra.config import Config | |
| config = Config() | |
| if not getattr(config, "debug_save_horizons_output", False): | |
| return | |
| horizons_dir = config.paths.logs / "horizons" | |
| horizons_dir.mkdir(parents=True, exist_ok=True) | |
| safe_name = body_name.replace(" ", "_").replace("/", "_") | |
| output_path = horizons_dir / f"{safe_name}_{context}_{datetime.utcnow().strftime('%Y%m%dT%H%M%S%f')}.ecsv" | |
| eph.write(output_path, format="ascii.ecsv", overwrite=True) | |
| logger.debug("Saved raw Horizons output for %s (%s) to %s", body_name, context, output_path) | |
| except Exception as exc: | |
| logger.warning("Failed to save raw Horizons output for %s (%s): %s", body_name, context, exc) | |
| try: | |
| logger.debug("Horizons API call input for %s (%s): %s", body_name, context, call_input) |
| # If body provided, get ra/dec | ||
| if lookup_name is not None: | ||
| if "Telescope" in paired_devices: | ||
| # Get observatory location from telescope (cached) | ||
| telescope_name = paired_devices["Telescope"] | ||
| obs_location = self.get_observatory_location(telescope_name) | ||
|
|
||
| # Get current time | ||
| now = Time.now() | ||
|
|
||
| # Get body coordinates | ||
| target_coord = astra.utils.get_body_coordinates( | ||
| body_name=lookup_name, | ||
| obs_time=now, | ||
| obs_location=obs_location, | ||
| tle=action_value.get("tle"), | ||
| near=near | ||
| ) |
There was a problem hiding this comment.
When non-sidereal tracking is active you already have precomputed interpolators on the action config, and slew_target_radec_deg overrides the actual slew target. However, setup_observatory still calls get_body_coordinates(..., near=True) which can trigger an extra Horizons network call for minor bodies/TLEs (and extra logging/disk writes) even though the result is not used for slewing. Consider skipping this lookup when slew_target_radec_deg is provided, or deriving the “current” RA/Dec from the stored interpolator instead of calling Horizons again.
name: Pull request
Pull Request
Description
This PR builds upon the non-sidereal framework introduced in #130 to enable direct tracking of Earth satellites via Two-Line Element (TLE) datasets. By extending the ephemeris and coordinate utilities to process TLE formats directly strings, Astra can now dynamically compute and apply the extreme differential tracking rates required for passing satellites in real-time.
Changes Made
How to Test
Checklist