-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Add spherical circles and polygons? #276
Comments
See #303; it would be helpful both in building client-side interfaces to IVOA services, but especially in implementing IVOA protocols in Python, if these spherical geometry region types existed, as they map directly to concepts in the IVOA interfaces. |
The question how to represent and handle "true sky" regions that work without a WCS has come up many times, see e.g. #13, #295, #303, #76, #221 Ideas how to handle this included a flag (e.g. I don't think there's a perfect solution, because the most-used serialisation format (DS9) doesn't support the distinction, and there is no great default behaviour. E.g. I think most users would expect CircleSkyRegion to be spherical, but it's planar. Making it spherical would mean that one has to go to polygon on My suggestion would be to start by adding new classes for the spherical cases:
It's contains would look like this, and it could or couldn't sub-class
I'm not sure how DS9 or other serialisation should behave. I'll send a PR today for the |
I think it'll be necessary to have some docstring and/or actual documentation to help users understand the difference between circles and spherical circles. It's not at all intuitive. I think the most useful piece of imagery is to make sure that the reader thinks about a planar circle that's away from the point of tangency of a tangent plane. That helps (I think) break the "but how can they be different - they're both rotationally symmetric, so they must be the same" conceptual block. |
I started to implement separate classes in #306 . But now I've left astronomy and am starting a new job, so I'll close that PR now. @gpdf @larrybradley @keflavich @astrofrog or anyone - if you're interested to add spherical regions, and if you want to pursue the separate classes approach, you could start a new PR from that branch. If you prefer another way, e.g. adding methods and possibly a flag to existing classes, it's probably better if you start from master. |
I came across this package recently with an interest in finding sources within irregular regions. It seems like this issue defines the solution most clearly. Since there is not a lot of activity on this issue, I'm wondering if people here can recommend a workaround.
I would think that I could do this well enough with |
This is a reminder issue that we might want to bring back spherical circles and polygons in the regions package.
We had at least spherical circle functionality at some point, but for consistency with DS9 and to avoid confusion of having one class represent a planar and a spherical sky circle sometimes, it was removed in #132 .
Now in #275 I'm removing the last trace of that functionality, an unused and untested helper function. But really, I think we probably should bring that functionality back (with a minor change, not call
numpy.matrix
, but instead use arrays andnumpy.dot
ornumpy.matmul
). I think it was discussed before, and we thought a separate class likeCircleSphericalSkyRegion
in addition to CircleSkyRegion would be the way to go?@astrofrog - Do you remember?
The text was updated successfully, but these errors were encountered: