DM-45752: Support region overlap against points #1054
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1054 +/- ##
==========================================
+ Coverage 89.51% 89.53% +0.02%
==========================================
Files 361 361
Lines 46147 46200 +53
Branches 9460 9469 +9
==========================================
+ Hits 41309 41366 +57
+ Misses 3504 3499 -5
- Partials 1334 1335 +1 ☔ View full report in Codecov by Sentry. |
| case lsst.sphgeom.LonLat(): | ||
| return _make_region_literal_from_lonlat(value) | ||
| case astropy.coordinates.SkyCoord(): | ||
| icrs = value.transform_to("icrs") |
There was a problem hiding this comment.
Is ICRS the right coordinate system?
| ra_value = _get_float_literal_value(ra, node.ra) | ||
| dec_value = _get_float_literal_value(dec, node.dec) | ||
|
|
||
| lon_lat = lsst.sphgeom.LonLat.fromDegrees(ra_value, dec_value) |
There was a problem hiding this comment.
Did you notice whether those preexisting docs specifically said the values were supposed to be decimal degrees? If we don't want to get into handling multiple angular units (to say nothing of H:M:S angles), decimal degrees is probably the best choice, but it definitely needs to be documented.
There was a problem hiding this comment.
The only thing it previously said about units was "similar to ADQL syntax", which implies decimal degrees.
I had already tweaked the docs to say this:
You can check for overlap of a region with a point using the
POINT(ra, dec)syntax,
whereraanddecare specified as an ICRS sky position in degrees.
Is that a good way to phrase it?
| case lsst.sphgeom.LonLat(): | ||
| vec = lsst.sphgeom.UnitVector3d(value) | ||
| # Convert the point to a Region by representing it as a zero-radius | ||
| # Circle. |
There was a problem hiding this comment.
I wasn't sure whether zero-radius would work, but if it does, great!
There was a problem hiding this comment.
It seems to, and from the docs it seems to be intended to with slight caveats (e.g. they go into a discussion of an edge case where two zero-radius circles that are very close but not exact might still be considered overlapping.)
| icrs = value.transform_to("icrs") | ||
| ra = icrs.ra.degree | ||
| dec = icrs.dec.degree | ||
| lon_lat = lsst.sphgeom.LonLat.fromDegrees(ra, dec) |
There was a problem hiding this comment.
astropy.coordinates.SkyCoord can be array-valued (which I think means that icrs.ra.degree and icrs.dec.degree will come back as something numpy-based. We should probably guard against this to make the exception raised friendlier.
dhirving
force-pushed
the
tickets/DM-45752
branch
from
8a687ba
to
9e4197e
Compare
Spatial queries using the new query system can now be constrained using the `POINT()` function in `where` strings. This has long been documented to exist, but never worked.
Region overlaps can now be tested against points specified as `lsst.sphgeom.LonLat` or `astropy.coordinates.SkyCoord` values.
Added more information about region overlap queries . Removed information about "IN" syntax for regions -- this has never been supported.
astropy's SkyCoord can be array-valued, but when specifying a point we can only use a scalar. We now raise a better error message when this occurs.
dhirving
force-pushed
the
tickets/DM-45752
branch
from
9e4197e
to
659e30c
Compare
Region overlap queries can now use points as regions. Points can be specified
as
region OVERLAPS POINT(ra, dec), or by binding anlsst.sphgeom.LonLatorastropy.coordinates.SkyCoordvalue. (At the moment, this feature is onlyavailable when using client/server Butler.)
Checklist
doc/changesconfigs/old_dimensions