Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Ellipse translation in app.get_subsets() is wrong #2241

Closed
pllim opened this issue Jun 6, 2023 · 0 comments · Fixed by #2240 or #2244
Closed

BUG: Ellipse translation in app.get_subsets() is wrong #2241

pllim opened this issue Jun 6, 2023 · 0 comments · Fixed by #2240 or #2244
Labels
bug Something isn't working imviz

Comments

@pllim
Copy link
Contributor

pllim commented Jun 6, 2023

roi_as_region = EllipsePixelRegion(PixCoord(xc, yc), rx, ry, Angle(theta, "deg"))

is wrong because glue defines radius while regions defines width and height. Really, we should be using the translators already vetted and in place here: https://github.com/glue-viz/glue-astronomy/blob/main/glue_astronomy/translators/regions.py

The complication is that to use the upstream translator directly the way we do the translation in this method, we also need to modify upstream.

I already have the fixes but I open an issue anyway so users are aware, because this bug is very subtle. Fixes here:

🐱

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working imviz
Projects
None yet
1 participant