Closed pford closed 2 months ago
@pford Thanks for addressing the issues I found. All the four issues are fixed. There is one last thing: when exporting an annotation ellipse, the position angle seems having a small offset. this branch: ann ellipse [[17:56:21.28865, -021.57.23.2793], [4.0851arcsec, 2.5532arcsec], 0.00000250deg] coord=J2000, corr=[I], linewidth=2, linestyle=-, symsize=1, symthick=1, color=ffba01, font=Helvetica, fontsize=10, fontstyle=bold, usetex=false
dev: ann ellipse [[17:56:21.28865, -021.57.23.2793], [4.0851arcsec, 2.5532arcsec], 0.00000000deg] coord=J2000, corr=[I], linewidth=2, linestyle=-, symsize=1, symthick=1, color=ffba01, font=Helvetica, fontsize=10, fontstyle=bold, usetex=false
@kswang1029 I tested this branch pam/1347_region_converter
and the combined branch pam/combine_1347_1339_branches
on Ubuntu and macOS 13.6, exporting ann ellipse to CRTF World in the reference image and a matched image. In all cases, the angle was 0.00000000deg.
@kswang1029 I tested this branch
pam/1347_region_converter
and the combined branchpam/combine_1347_1339_branches
on Ubuntu and macOS 13.6, exporting ann ellipse to CRTF World in the reference image and a matched image. In all cases, the angle was 0.00000000deg.
Odd. I will do some extra multi-platform tests.
@pford I tried the following platforms but got exactly the same results (thus test failure):
What's your test platform? Any relevant libraries that we should compare their versions?
meanwhile I will check test workflow.
@pford I spotted a behavior about the PA of ellipse and ann ellipse. When we make it as "oblate", the PA is 0 deg. However, if we make it as "prolate", there will be a small shift in PA.
Could you see if you can reproduce this in your env please?
I confirm that with v4.1 PA is still 0deg.
@kswang1029 fixed by using double instead of float for angle in ellipse record. More accurate when converted from rad to deg.
Package | Line Rate | Health |
---|---|---|
src.Cache | 72% | ➖ |
src.DataStream | 44% | ➖ |
src.FileList | 67% | ➖ |
src.Frame | 36% | ❌ |
src.HttpServer | 42% | ➖ |
src.ImageData | 28% | ❌ |
src.ImageFitter | 83% | ✔ |
src.ImageGenerators | 43% | ➖ |
src.ImageStats | 75% | ✔ |
src.Logger | 37% | ❌ |
src.Main | 52% | ➖ |
src.Region | 69% | ➖ |
src.Session | 4% | ❌ |
src.Table | 52% | ➖ |
src.ThreadingManager | 67% | ➖ |
src.Timer | 85% | ✔ |
src.Util | 40% | ➖ |
Summary | 46% (8589 / 18738) | ➖ |
Description
What is implemented or fixed? Mention the linked issue(s), if any. Implements #1347
How does this PR solve the issue? Give a brief summary.
Are there any companion PRs (frontend, protobuf)? No, backend change only
Is there anything else that testers should know (e.g. exactly how to reproduce the issue)? The behavior should be unchanged by this refactor, except in small underlying details (e.g. LCRegion::toRecord returns the coordinates 1-based but creating the Record from control points is 0-based, as specified in the Record's "oneRel" field).
Checklist