search

desihub / desisurvey

Code for desi survey planning and implementation
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

Updates for astropy 6. #158

Closed schlafly closed 6 months ago

schlafly commented 6 months ago

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 commented 6 months ago

This is intended to resolve https://github.com/desihub/desisurvey/issues/157 .

schlafly commented 6 months ago

@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.

coveralls commented 6 months ago

Coverage Status

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

sbailey commented 6 months ago

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.

weaverba137 commented 6 months ago

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.