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

Support for three IVOA region types as astropy.regions objects: what to do about RANGE? #303

Open
gpdf opened this issue Oct 14, 2019 · 4 comments

Comments

@gpdf
Copy link

gpdf commented Oct 14, 2019

In implementing a server for some of the IVOA protocols, it is necessary to be able to represent three basic geometric regions in Python: a "circle" (defined by a sky position and a radius), a "polygon" (defined by a series of sky positions), and a "range" (defined as a region with RA within a specified range and DEC within a specified range).

I would love to use Astropy.regions types in my implementation so that the interface will be familiar to Astropy users.

It looks like if the spherical circle and spherical polygon regions foreseen in #276 were implemented, they would map well to the IVOA types.

However, note that "range" as defined is not a spherical polygon. It looks therefore like a third new class might be necessary. An additional complication is that the IVOA "range" concept includes the possibility of either the RA dimension or the DEC direction being unbounded, i.e., representing the entire sky. In the IVOA-mandated serialization of the object, this is represented by having the coordinate ranges extend to +/-Inf, as appropriate. See the SIAv2 standard below.

References:

@cdeil
Copy link
Member

cdeil commented Oct 15, 2019

@gpdf - Thanks for bringing this up.

We have RectangleSkyRegion already which corresponds to the box region type in DS9, but it's not a "pure spherical" region that can be evaluated without a WCS. Also with the special case that you mention of using +- INF to support all-sky, that's not foreseen, and the parametrisation is center, width, height, and not lon and lat min / max box.

@gpdf @larrybradley - Should we try to cover this "range" use case with the "box" region, or add a new separate "range" region?

@gpdf - Are you interested to send a PR for this (either improved code & tests & docs for rectangle, or a new range region), or should we do it?

@keflavich
Copy link
Contributor

I'd say we need a new range region - box implies bounded on all sides, and the existing serializations of rectangles have no concept of +/-inf.

@gpdf
Copy link
Author

gpdf commented Oct 15, 2019

I haven't tried reading the code for RectangleSkyRegion carefully. I was assuming that it was going to end up being a spherical rectangle (i.e., a four-sided polygon with great circle edges with pairs of opposing equal-length sides and equal angles) and not a long/lat region.

ADQL's "BOX" is notoriously not a long/lat region (and is likely to be dropped from ADQL for being confusing and therefore difficult to implement correctly).

One way to think about it is: the shape of a spherical rectangle is the same no matter where it is on the sky, while long-lat regions have different shapes depending on latitude. Think of the region ( long in (0, 90), lat in (89,89.5) ) - that's not a spherical rectangle!

So I think a "range" region would be appropriate.

I would be willing to consider coding it but from reading the existing regions I am not entirely clear on what interfaces a new region has to satisfy. Is just coding to the SkyRegion's base class interface sufficient?

Is this the same interface that would be used for the spherical circle, polygon, and rectangle from #276, then? Would you (plural) still be implementing those?

@cdeil
Copy link
Member

cdeil commented Oct 23, 2019

I think the problem isn't actually about who types up the code. We'd love to have you as contributor of course, but I'm also willing to do that in the next days. In the end it'll be ~ 30 lines of simple Python code to handle the range region, and maybe including tests and docs a day of work.The problem is rather that it's difficult to decide how this should work in detail. @gpdf - please see #276 (comment) and share your thoughts and experience!

@larrybradley larrybradley modified the milestone: 0.5 Jul 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants