Skip to content

Fix mistake converting string date during leap second to jd1, jd2#51

Merged
taldcroft merged 1 commit intomasterfrom
fix-fast-convert-leap-sec
Apr 15, 2025
Merged

Fix mistake converting string date during leap second to jd1, jd2#51
taldcroft merged 1 commit intomasterfrom
fix-fast-convert-leap-sec

Conversation

@taldcroft
Copy link
Copy Markdown
Member

@taldcroft taldcroft commented Mar 30, 2025

Description

The function convert_string_to_jd1_jd2 mistakenly used the TT scale for conversion. I do not understand where that came from, but UTC is the right answer because all the Chandra string-based date formats with fast converter support are UTC.

This only impacts conversions for times within a leap second. The last leap second was 2016-12-31, so this issue has almost no impact, but still good to fix.

Interface impacts

Testing

Unit tests

  • Mac
(ska3) ➜  cxotime git:(fix-fast-convert-leap-sec) git rev-parse --short HEAD                   
c7d7cbe
(ska3) ➜  cxotime git:(fix-fast-convert-leap-sec) pytest
================================================== test session starts ===================================================
platform darwin -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /Volumes/git
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 290 items                                                                                                      

cxotime/tests/test_cxotime.py .................................................................................... [ 28%]
.................................................................................................................. [ 68%]
.............................................................................                                      [ 94%]
cxotime/tests/test_cxotime_linspace.py ...............                                                             [100%]

================================================== 290 passed in 2.47s ===================================================

Independent check of unit tests by Jean

  • Linux
ska3-jeanconn-fido> git rev-parse HEAD
c7d7cbe865ce9491d02c58550ebf0d1f5d40348a
ska3-jeanconn-fido> pytest
============================= test session starts ==============================
platform linux -- Python 3.12.8, pytest-8.3.4, pluggy-1.5.0
rootdir: /proj/sot/ska/jeanproj/git
configfile: pytest.ini
plugins: anyio-4.7.0, timeout-2.3.1
collected 290 items                                                                                                       

cxotime/tests/test_cxotime.py .......................................... [ 14%]
........................................................................ [ 39%]
........................................................................ [ 64%]
........................................................................ [ 88%]
.................                                                                                                   [ 94%]
cxotime/tests/test_cxotime_linspace.py ...............                                                              [100%]

=================================================== 290 passed in 5.58

Functional tests

New unit test.

@taldcroft taldcroft changed the title Fix mistake converting string date to jd1, jd2 Fix mistake converting string date during leap second to jd1, jd2 Apr 3, 2025
@taldcroft taldcroft requested review from javierggt and jeanconn April 3, 2025 10:49
@jeanconn
Copy link
Copy Markdown
Contributor

jeanconn commented Apr 3, 2025

By "because all the Chandra date formats are UTC" do you mean everything except Chandra seconds or do you not consider that a date format or?

@taldcroft
Copy link
Copy Markdown
Member Author

By "because all the Chandra date formats are UTC" do you mean everything except Chandra seconds or do you not consider that a date format or?

I updated the description "because all the Chandra string-based date formats with fast converter support are UTC."

@taldcroft taldcroft removed the request for review from javierggt April 14, 2025 10:53
Copy link
Copy Markdown
Contributor

@jeanconn jeanconn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I remain not an expert on leap second handling in erfa, but this fix also makes sense in that it repairs the symmetry with convert_string_to_jd2_jd2 (which explicitly uses dtf2d with UTC scale). I also explicitly confirmed that the new test fails in master as expected.

@taldcroft taldcroft merged commit d64917a into master Apr 15, 2025
2 checks passed
@taldcroft taldcroft deleted the fix-fast-convert-leap-sec branch April 15, 2025 10:41
This was referenced Jun 13, 2025
@taldcroft taldcroft restored the fix-fast-convert-leap-sec branch September 25, 2025 18:38
@taldcroft taldcroft deleted the fix-fast-convert-leap-sec branch September 25, 2025 18:39
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