fixed bug in coordinate transformation #997
Conversation
- units in camera frame were not always taken into account - added unit test to check correct handling of units
|
|
||
| # first define the camera frame | ||
| pointing = SkyCoord(alt=70*u.deg, az=0*u.deg,frame=AltAz()) | ||
| camera_frame = CameraFrame(focal_length=focal_length) |
| altaz_coord = camera_coord.transform_to(AltAz()) | ||
|
|
||
| # transform back | ||
| altaz_coord2 = SkyCoord(az=altaz_coord.az, alt=altaz_coord.alt, frame=hf) |
There was a problem hiding this comment.
hf is undefined (should be defined above or just add AltAz() here.
| camera_coord2 = altaz_coord2.transform_to(camera_frame) | ||
|
|
||
| # check transform | ||
| assert camera_coord.x == camera_coord2.x |
There was a problem hiding this comment.
Better use assert np.isclose(camera_coord.x.to_value(u.m), camera_coord2.to_value(u.m), comparing floats for equality is often not a good idea.
- pointing in camera frame definition - defined frame - checking with 'is close' instead of 'is equal'
|
The tests are still failing (missing imports), you can run them locally using |
But this seems unrelated to my added code |
It fails in the test you added. You need to import stuff in the test function |
|
Add the imports after: And maybe call it |
- camera_coord.y.to_value instead of camera_coord.to_value
Codecov Report
@@ Coverage Diff @@
## master #997 +/- ##
==========================================
+ Coverage 78.93% 78.95% +0.02%
==========================================
Files 195 195
Lines 11328 11340 +12
==========================================
+ Hits 8942 8954 +12
Misses 2386 2386
Continue to review full report at Codecov.
|
|
Just code quality not up to standards. Check the output with "Details" to see what has to be changed. |
|
I would Merge regardless |
When transforming from horizon frame to camera frame and back, it gives different results.
When specifying pixels in meters, everything is fine, but not for millimetres for example.
Changed
u.Quantity((x_rotated / focal_length).to_value(), u.rad)to
delta_alt = u.Quantity((x_rotated / focal_length).to_value(u.dimensionless_unscaled), u.rad)Added unit test to check correct handling of units.