Skip to content

Fix GPS-to-UTC conversions immediately before leap-second insertions - #201

Open
heathsanchez wants to merge 3 commits into
Stanford-NavLab:v1.1.1from
metalogiclabs:heathsanchez/fix-gps-utc-leap-second-v111
Open

heathsanchez wants to merge 3 commits into
Stanford-NavLab:v1.1.1from
metalogiclabs:heathsanchez/fix-gps-utc-leap-second-v111

Conversation

@heathsanchez

Copy link
Copy Markdown

Summary

Fix the one-second GPS-to-UTC conversion error immediately before positive leap-second insertions. The existing tow_to_datetime() selects a leap-second offset by comparing a GPS-clock timestamp against UTC transition dates, applying the new offset prematurely.

This change:

  • Selects the offset using the corresponding transition on the GPS-clock timescale.
  • Preserves the public API, get_leap_seconds() behavior, and existing dependencies.
  • Preserves the existing lossy convention for the inserted leap second; Python datetime cannot represent 23:59:60, and this PR does not introduce that representation.

Reproduction

Before the fix:

utc = datetime(2016, 12, 31, 23, 59, 59, tzinfo=timezone.utc)
week, tow = datetime_to_tow(utc)  # 1930, 16.0
tow_to_datetime(week, tow)       # Incorrectly returns 23:59:58 UTC

After the fix, the inverse returns 2016-12-31 23:59:59 UTC. Dependent public datetime and Unix-millisecond conversions also produce the expected values.

Tests and evidence

  • Tested all 18 recorded positive leap-second insertions, with 30 ordinary UTC timestamps immediately before each (540 cases). Incorrect round trips dropped from 153 on the original release snapshot to zero with this patch.
  • Added representative 1982, 2015, and 2016 round-trip regressions; independently specified expected values for four public conversion paths; and tests for the inserted-second mapping and pre-GPS-epoch rejection.
  • Added a synthetic downstream prepare_kaggle_submission() regression checking UnixTimeMillis. No claim is made that real competition data was affected.
  • Local Python 3.12 offline suite: 690 passed, 4 deselected, 2 dependency deprecation warnings. The four deselected tests require external FTP/HTTP; upstream CI remains to be verified.
  • Coverage compared with the original release-branch snapshot: 97.6816% → 97.6870% overall; 100% in time_conversions.py.
  • Pylint 2.17.7: 10.00/10 for the modified production file. A Python-3.12-compatible lint comparison found no new errors or warnings in the touched files.
  • git diff --check passed.

This PR targets the current v1.1.1 release branch, following the contributing guide. There are no API changes, new dependencies, or directory changes.

Fixes #192

AI assistance: OpenAI Codex helped reconstruct and independently test the change.

Select leap-second offsets on the GPS-clock timeline while preserving the existing inserted-second mapping. Add historical, public API, and downstream export regressions.

AI assistance: OpenAI Codex helped reconstruct and test this change.
@codecov

codecov Bot commented Nov 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (v1.1.1@3ca808f). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             v1.1.1     #201   +/-   ##
=========================================
  Coverage          ?   99.61%           
=========================================
  Files             ?       27           
  Lines             ?     3908           
  Branches          ?        0           
=========================================
  Hits              ?     3893           
  Misses            ?       15           
  Partials          ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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