feat: add fetch_earth_orientation_parameters to eop in celerity module - #239
feat: add fetch_earth_orientation_parameters to eop in celerity module#239michealroberts wants to merge 1 commit into
Conversation
michealroberts
added
enhancement
feat: add fetch_earth_orientation_parameters to eop in celerity module
706ec1c to
dfc1a83
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds an Earth Orientation Parameters (EOP) fetch capability to celerity.eop, introducing HTTP requests to the IERS EOP REST endpoint and returning a typed EOP record.
Changes:
- Added EOP fetching utilities (
parse_eop_value,fetch_eop_parameter,fetch_earth_orientation_parameters) insrc/celerity/eop.py. - Added constants for IERS EOP endpoint configuration and LOD fallback behavior.
- Reformatted
tests/test_eop.pywith additional section separators.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
src/celerity/eop.py |
Implements IERS-backed fetching/parsing of multiple EOP parameters with an LOD backfill strategy. |
tests/test_eop.py |
Adds separator comments between tests; no new behavioral assertions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if value is None: | ||
| return None | ||
|
|
||
| if isinstance(value, int | float): |
There was a problem hiding this comment.
isinstance(value, int | float) will raise TypeError at runtime because isinstance does not accept PEP 604 union types. Use a tuple of types instead (e.g., (int, float)) to keep this function working on all supported Python versions.
| if isinstance(value, int | float): | |
| if isinstance(value, (int, float)): |
| def fetch_eop_parameter( | ||
| param: Literal["UT1-UTC", "x_pole", "y_pole", "LOD", "dX", "dY"], MJD: int | ||
| ) -> float | None: | ||
| query: EOPRequestParams = EOPRequestParams( | ||
| { | ||
| "param": param, | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ) |
There was a problem hiding this comment.
This module constructs EOP request URLs with urlencode(query) but doesn’t apply the repo’s existing encoding approach for the series parameter (spaces). Elsewhere (e.g., celerity.temporal.get_ut1_utc_offset) the URL is built with urlencode(..., safe=' ') and then + is replaced with %20 to avoid server-side issues with space handling. Consider reusing that pattern (or a shared helper) here for consistency and to reduce the chance of IERS rejecting/altering the query.
| # Assume UTF-8 or ASCII text in the response: | ||
| raw = response.read().decode("utf-8", errors="ignore") | ||
|
|
||
| # Load the JSON data from the response: | ||
| data = loads(raw) | ||
|
|
||
| if int(data["MJD"]) != MJD: | ||
| raise ValueError( | ||
| f"Received MJD {data['MJD']} does not match requested MJD {MJD}" | ||
| ) | ||
|
|
||
| if data["Param"] != param: | ||
| raise ValueError( | ||
| f"Received parameter {data['Param']} does not match requested parameter {param}" |
There was a problem hiding this comment.
fetch_eop_parameter assumes the JSON response always contains MJD, Param, and Value. If the service returns an error payload, an empty response, or a schema change, this will raise KeyError/TypeError rather than a clear ValueError. The existing fetch_iers_rapid_service_data helper performs explicit key/None checks and raises actionable errors; consider adding similar validation here (or routing requests through that helper).
| # ************************************************************************************** | ||
|
|
||
|
|
||
| def fetch_earth_orientation_parameters(MJD: int) -> EarthOrbitalParameters: | ||
| # Setup the query parameters for the IERS Rapid Service data: | ||
| queries: List[EOPRequestParams] = [ | ||
| EOPRequestParams( | ||
| { | ||
| "param": "UT1-UTC", | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ), | ||
| EOPRequestParams( | ||
| { | ||
| "param": "x_pole", | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ), | ||
| EOPRequestParams( | ||
| { | ||
| "param": "y_pole", | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ), | ||
| EOPRequestParams( | ||
| { | ||
| "param": "LOD", | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ), | ||
| EOPRequestParams( | ||
| { | ||
| "param": "dX", | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ), | ||
| EOPRequestParams( | ||
| { | ||
| "param": "dY", | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ), | ||
| ] | ||
|
|
||
| entry: EarthOrbitalParameters = EarthOrbitalParameters( | ||
| { | ||
| "mjd": float(MJD), | ||
| "dut1": -inf, | ||
| "x_polar_motion": -inf, | ||
| "y_polar_motion": -inf, | ||
| "lod": -inf, | ||
| "pole_offset_in_ecliptic_longitude": -inf, | ||
| "pole_offset_in_ecliptic_obliquity": -inf, | ||
| } | ||
| ) | ||
|
|
||
| for q in queries: | ||
| value = fetch_eop_parameter(q["param"], MJD) | ||
|
|
||
| if value is None: | ||
| continue | ||
|
|
||
| if q["param"] == "UT1-UTC": | ||
| # Convert our DUT1 value from milliseconds to seconds: | ||
| entry["dut1"] = value * 0.001 | ||
|
|
||
| if q["param"] == "x_pole": | ||
| entry["x_polar_motion"] = convert_arcseconds_to_degrees(value / 1000.0) | ||
|
|
||
| if q["param"] == "y_pole": | ||
| entry["y_polar_motion"] = convert_arcseconds_to_degrees(value / 1000.0) | ||
|
|
||
| if q["param"] == "LOD": | ||
| # Convert LOD from milliseconds to seconds. | ||
| entry["lod"] = value * 0.001 | ||
|
|
||
| if q["param"] == "dX": | ||
| entry["pole_offset_in_ecliptic_longitude"] = convert_arcseconds_to_degrees( | ||
| value / 1000.0 | ||
| ) | ||
|
|
||
| if q["param"] == "dY": | ||
| entry["pole_offset_in_ecliptic_obliquity"] = convert_arcseconds_to_degrees( | ||
| value / 1000.0 | ||
| ) | ||
|
|
||
| # IAU2000 predictions do not publish LOD, so backfill from the nearest prior MJD: | ||
| if entry["lod"] == -inf: | ||
| for fallback_mjd in range(MJD - 1, MJD - EOP_MAX_LOD_LOOKBACK_DAYS - 1, -1): | ||
| value = fetch_eop_parameter("LOD", fallback_mjd) | ||
|
|
||
| if value is None: | ||
| continue |
There was a problem hiding this comment.
fetch_earth_orientation_parameters can make a large number of network calls: 6 requests for the main parameters plus up to EOP_MAX_LOD_LOOKBACK_DAYS (365) additional requests when backfilling LOD. This is likely to be very slow and can unintentionally hammer the IERS service (rate limiting / blocking). Consider adding caching (similar to celerity.iers), reducing the number of requests (e.g., only query LOD fallbacks when needed and cache negative results), or fetching parameters in a single request if the API supports it.
|
|
||
| entry["lod"] = value * 0.001 | ||
| break | ||
|
|
There was a problem hiding this comment.
fetch_earth_orientation_parameters initializes all fields to -inf and silently returns those sentinels when a parameter is missing (Value is None/"-") or when LOD backfill fails within the lookback window. Returning -inf for physical quantities (seconds/degrees) makes downstream computations fail in non-obvious ways. Consider raising a ValueError when required parameters can’t be fetched, or change the return type to make missing values explicit (e.g., float | None).
| # Ensure that we do not silently return sentinel values for required parameters. | |
| missing_params = [ | |
| name | |
| for name in ( | |
| "dut1", | |
| "x_polar_motion", | |
| "y_polar_motion", | |
| "lod", | |
| "pole_offset_in_ecliptic_longitude", | |
| "pole_offset_in_ecliptic_obliquity", | |
| ) | |
| if entry.get(name, -inf) == -inf | |
| ] | |
| if missing_params: | |
| raise ValueError( | |
| f"Earth orientation parameters missing for MJD {MJD}: " | |
| + ", ".join(missing_params) | |
| ) |
| ) | ||
|
|
||
| for q in queries: | ||
| value = fetch_eop_parameter(q["param"], MJD) |
There was a problem hiding this comment.
The queries list stores full EOPRequestParams dicts (including mjd and series), but the loop only uses q["param"] and always passes the outer MJD into fetch_eop_parameter. This adds duplication and makes it easier for future edits to accidentally diverge. Consider simplifying queries to a list of parameter names, or have the loop pass q["mjd"]/q["series"] through so the data structure is actually used.
| value = fetch_eop_parameter(q["param"], MJD) | |
| value = fetch_eop_parameter(q["param"], q["mjd"]) |
| # ************************************************************************************** | ||
|
|
||
|
|
||
| def parse_eop_value(value: object) -> float | None: | ||
| if value is None: | ||
| return None | ||
|
|
||
| if isinstance(value, int | float): | ||
| return float(value) | ||
|
|
||
| raw = str(value).strip() | ||
|
|
||
| if raw in {"", "-"}: | ||
| return None | ||
|
|
||
| return float(raw) | ||
|
|
||
|
|
||
| # ************************************************************************************** | ||
|
|
||
|
|
||
| def fetch_eop_parameter( | ||
| param: Literal["UT1-UTC", "x_pole", "y_pole", "LOD", "dX", "dY"], MJD: int | ||
| ) -> float | None: | ||
| query: EOPRequestParams = EOPRequestParams( | ||
| { | ||
| "param": param, | ||
| "mjd": MJD, | ||
| "series": "Finals All IAU2000", | ||
| } | ||
| ) | ||
|
|
||
| url: str = f"{IERS_EOP_BASE_URL}?{urlencode(query)}" | ||
|
|
||
| # Ensure we always expect to accept JSON responses, whilst also letting the server | ||
| # know that we are a client (e.g., celerity) to avoid any potential issues with | ||
| # server-side rate limiting or blocking: | ||
| request = Request( | ||
| url, | ||
| headers={ | ||
| "Accept": "application/json", | ||
| "User-Agent": "celerity", | ||
| }, | ||
| ) | ||
|
|
||
| with urlopen(request, timeout=EOP_TIMEOUT_SECONDS) as response: | ||
| # Assume UTF-8 or ASCII text in the response: | ||
| raw = response.read().decode("utf-8", errors="ignore") | ||
|
|
||
| # Load the JSON data from the response: | ||
| data = loads(raw) | ||
|
|
||
| if int(data["MJD"]) != MJD: | ||
| raise ValueError( | ||
| f"Received MJD {data['MJD']} does not match requested MJD {MJD}" | ||
| ) | ||
|
|
||
| if data["Param"] != param: | ||
| raise ValueError( | ||
| f"Received parameter {data['Param']} does not match requested parameter {param}" | ||
| ) | ||
|
|
||
| return parse_eop_value(data["Value"]) | ||
|
|
||
|
|
||
| # ************************************************************************************** | ||
|
|
There was a problem hiding this comment.
New network-facing behavior (parse_eop_value, fetch_eop_parameter, fetch_earth_orientation_parameters) isn’t covered by tests. The repo already has tests exercising IERS fetching/caching behavior (see tests/test_iers.py), so adding tests here would help prevent regressions (e.g., missing-value handling, unit conversions, and LOD backfill behavior).
| EOP_TIMEOUT_SECONDS = 10 | ||
|
|
||
| # ************************************************************************************** | ||
|
|
||
| EOP_MAX_LOD_LOOKBACK_DAYS = 365 | ||
|
|
||
| # ************************************************************************************** |
There was a problem hiding this comment.
This file duplicates IERS constants/behavior that already exist in celerity.iers (e.g., IERS_EOP_BASE_URL, timeout, request headers, and caching). To reduce drift and keep all IERS interaction consistent (encoding, error handling, caching), consider importing/reusing the existing IERS helper(s) instead of reimplementing another fetch path here.
feat: add fetch_earth_orientation_parameters to eop in celerity module