WCS and alignment operations fixed for datacubes#2147
Conversation
Allow manual triggering of CI workflow
kbwestfall
left a comment
kbwestfall
left a comment
There was a problem hiding this comment.
Thanks, @rcooke-ast ! Two main comments from me:
- Can we split up the new datacube test?
- Do we need the new
linear_interpolate_extrapolatefunction?
The rest are some code clean-up comments.
| assert np.isclose(ra_offsets[0].to(u.arcsec).value, 0.0, atol=atol) | ||
| assert np.isclose(dec_offsets[0].to(u.arcsec).value, 0.0, atol=atol) | ||
| assert np.isclose(ra_offsets[1].to(u.arcsec).value, offs_ra.to(u.arcsec).value, atol=atol) | ||
| assert np.isclose(dec_offsets[1].to(u.arcsec).value, offs_dec.to(u.arcsec).value, atol=atol) |
There was a problem hiding this comment.
Thanks so much for the new tests!! Can we break this single test function into multiple separate functions, or does the entire test need to run for it to make sense?
There was a problem hiding this comment.
Sure, I can break it up into three separate tests. That makes more sense. They are pseudo-independent, and it would be good to know if one crashes when another doesn't.
rcooke-ast
left a comment
rcooke-ast
left a comment
There was a problem hiding this comment.
Thanks @kbwestfall -- comments addressed!
| assert np.isclose(ra_offsets[0].to(u.arcsec).value, 0.0, atol=atol) | ||
| assert np.isclose(dec_offsets[0].to(u.arcsec).value, 0.0, atol=atol) | ||
| assert np.isclose(ra_offsets[1].to(u.arcsec).value, offs_ra.to(u.arcsec).value, atol=atol) | ||
| assert np.isclose(dec_offsets[1].to(u.arcsec).value, offs_dec.to(u.arcsec).value, atol=atol) |
There was a problem hiding this comment.
Sure, I can break it up into three separate tests. That makes more sense. They are pseudo-independent, and it would be good to know if one crashes when another doesn't.
3 participants
This is an extension to PR #1929 that fixes the alignment and WCS values discussed there. I will run the dev-suite to make sure I haven't broken everything, but all of the modes that I have tested work. This PR also includes a test (it's not ideal, because it doesn't touch the
coadd3dfile, but at least it's a fast way to confirm the core alignments codes). This addresses (I think!) Issue #2116.I have also made some coding improvements that increased the speed (by almost 2x) of creating datacubes.