Add generic imaging frame. #76
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #76 +/- ##
=======================================
Coverage ? 89.69%
=======================================
Files ? 8
Lines ? 941
Branches ? 0
=======================================
Hits ? 844
Misses ? 97
Partials ? 0 ☔ View full report in Codecov by Sentry. |
DanRyanIrish
force-pushed
the
imaging_frame
branch
from
1d9e83c to
bb74daa
Compare
Good for representing images from unknown instruments.
DanRyanIrish
force-pushed
the
imaging_frame
branch
from
bb74daa to
7fd3c6d
Compare
|
@samaloney could I get a review on this? |
xrayvision/coordinates/frames.py
Outdated
| PROJECTIVE_CTYPE_TO_UCD1 = { | ||
| "SXLT": "custom:pos.projective.lat", | ||
| "SXLN": "custom:pos.projective.lon", | ||
| "SXRZ": "custom:pos.projective.z", | ||
| } | ||
| astropy.wcs.wcsapi.fitswcs.CTYPE_TO_UCD1.update(PROJECTIVE_CTYPE_TO_UCD1) |
There was a problem hiding this comment.
This needs to be updated to match the CTYPE above but I think this part can be removed - have you made a plot with and without this included?
This is what I see
There was a problem hiding this comment.
Argh. Why is this? The representation is set to arcsec in L22-23 and L27-28. Where else does this need to be fixed?
There was a problem hiding this comment.
I think this is expected given the current code - specifically the ctype to UCD mapping might work now?
DanRyanIrish
force-pushed
the
imaging_frame
branch
from
ec9b296 to
a529872
Compare
DanRyanIrish
force-pushed
the
imaging_frame
branch
from
a529872 to
8a654db
Compare
|
@samaloney Is this now ready to merge? |
Without Ty/Tx, it causes conflicts when using STIXImaging. THerefore getting changing from Ty/Tx to lat/lon is a bigger change then shiould be done here.
DanRyanIrish
force-pushed
the
imaging_frame
branch
from
bf574c5 to
95bdde7
Compare
|
@samaloney I think all your comments have been resolved. |
This PR adds a generic imaging frame via a new coordinates module. This intention is that this frame would be used instead of HPC, which is currently used and is likely incorrect depending on the instrument which produced the visibilities.