Skip to content
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

Updates for astropy 6. #158

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Updates for astropy 6. #158

merged 1 commit into from
Apr 30, 2024

Conversation

schlafly
Copy link
Contributor

This changes some references to astropy.coordinates.ICRS to astropy.coordinates.ICRS() to reflect some astropy API changes.

It also loosens the tolerance to 40" from 30" for the match to some precomputed transformations. I don't understand why those transformations should have changed by 10", so maybe I should worry about this, but the old tolerance was 30" and the new tolerance is 40", so it's not obviously a big difference.

@schlafly
Copy link
Contributor Author

This is intended to resolve #157 .

@schlafly
Copy link
Contributor Author

@dkirkby , would you mind glancing at this PR? I suspect it's fine, but I needed to loosen the tolerance of the comparisons against JPL horizons to 40" from 30". Since I'm not doing any new math and that's not a lot, it doesn't really bother me. On the other hand, I might have expected the tolerances to be closer to 0.1" than 30" and so I can't decide if I should be more bothered.

@schlafly schlafly requested a review from dkirkby April 18, 2024 20:29
@coveralls
Copy link

Coverage Status

coverage: 30.413%. remained the same
when pulling 3463e31 on astropy-6
into fa29a9b on main.

@sbailey
Copy link
Contributor

sbailey commented Apr 30, 2024

I'm going to merge this and tag so that we have a working desisurvey + astropy/6.x for the 24.4 software release with Jura.

I agree with @schlafly 's surprise about 40" precision vs. 0.1" precision on these calculations though. Re-pinging @dkirkby for potential followup.

@sbailey sbailey merged commit c5df065 into main Apr 30, 2024
24 checks passed
@sbailey sbailey deleted the astropy-6 branch April 30, 2024 22:25
@weaverba137
Copy link
Member

I wonder if the previous tests were using a freeze_iers() calculation, whereas in Astropy 6, freeze_iers() is no longer necessary, since the required data files would be downloaded automatically as a separate Python package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants