New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update IERS_B file and adjust to new-style #14382
Conversation
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
Would probably need quick bugfix releases after this is merged? cc @astropy/astropy-project-release-team |
I guess users should not generally run into this, since we include the |
# | ||
# TODO: this test and the note can probably be removed after | ||
# enough time has passed that old-style IERS_B files are simply | ||
# not around any more, say in 2025. If so, also remove the excerpt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should open this TODO note as a proper GitHub issue and mention that issue here. Otherwise, we might forget. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it was OK as is, since there really isn't a hurry to deprecate...
Failures look related. I wasn't expecting changes... did they change the format and the actual data? |
Weird failures indeed! Checking the UT1-UTC one for
This is not the first time the "historical" data have changed -- I think they re-analyze once in a while. |
I think here we would just need to update the tests, but I'd prefer not to backport that, but rather have it only appear in 5.3. So, in #14384 we just update the |
Converted to draft so we do not merge this before #14384 is done. |
#14384 is merged. |
938dad8
to
ab61aa7
Compare
OK, I rebased and tried to make all the small changes that are required because the IERS B numbers have changed. |
I don't think this is a bug fix anymore. Move change log fragment to "API change"? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes LGTM though personally I am not impacted by these changes in my work. Who should co-review?
"It looks ok," Christian Bizouard (Observatoire de Paris / SYRTE) |
Needs a rebase with the partial revert merged. Thanks! |
ab61aa7
to
6c9a464
Compare
6c9a464
to
54e4ca4
Compare
Rebased. Also tried to rename the changelog fragment to |
"other" is only acceptable if it is not inside a subpackage folder, I think. EDIT: Yup, see https://github.com/astropy/astropy/blob/main/docs/changes/README.rst |
In case you haven't seen the memo, here it is:
|
This isn't IERS Bulletin B, btw. That is just preliminary monthly data. This is the final data except for the last 30 days, which gets an updated solution twice a week. My go to site for all of this is: |
Thanks for posting that! And, indeed, it isn't really IERS B, but well, at the time it seemed it was to me... (though the series that we actually download is not as "final" as you suggest or I thought; we've seen quite large parts changed, even if just by minimal amounts, of course, well within errors). |
That's interesting as the documentation says that only the last 30 days gets updated. C04.guide.pdf |
Yes, it is weird. See, e.g., #10376 |
But more importantly, @mkbrewer, does it make sense to you to shift to the new format with the new IRTF, and adjust tests accordingly? I guess the e-mail you quoted suggests we may not have much choice, though I think they have moved some things back because we contacted them (and perhaps others too). Mostly wondering if somewhere else we implicitly assume some version of the IRTF. |
Astropy doesn't assume anything about the ITRF. The IERS has just updated the frame is all. So, yes we need to adopt the new frame and adjust the tests accordingly. |
Can we push this forward or does this need more discussion/changes? Thanks! |
After @mkbrewer's confirmation that this is OK, I think we can just go ahead and merge! |
File was renamed in astropy#14382
File was renamed in astropy#14382
Astropy 5.3 introduced a new IERS_B table based on the ITRF2020 model of the Earth - see astropy/astropy#14382 for more details. The model came into effect in the IERS tables on 14 Feb 2023 and made it into Astropy 5.3 from there. The new frame definition causes a jump in UT1-UTC of about 0.05 ms, or an Earth rotation of about 1 mas, affecting topocentric coordinates like azel. Pick the Astropy 5.3 value as azel reference wherever it failed, and adjust the tolerance accordingly. The tests still pass on all minor versions of Astropy from 4.1 to 5.3.
Description
This pull request is to address the change in location and format of
IERS_B
data. Instructions are provided to read old-style files, but these are not automatically recognized since it should be very rare for the astropy version and the IERS_B file not to be synchronized.EDIT:
Fixes#14380 was fixed by, in the end, upstream reinstating the old IERS B tables.