Coordinate footprint crossmatch with a cutout size of 0 #166
Conversation
snbianco
force-pushed
the
zero-crossmatch
branch
from
d75ec79 to
acb143a
Compare
snbianco
force-pushed
the
zero-crossmatch
branch
from
acb143a to
7837082
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
==========================================
+ Coverage 96.25% 96.29% +0.04%
==========================================
Files 17 17
Lines 1787 1810 +23
==========================================
+ Hits 1720 1743 +23
Misses 67 67 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
astrocut/cutout.py
Outdated
|
|
||
| @staticmethod | ||
| def _parse_size_input(cutout_size): | ||
| def _parse_size_input(cutout_size, *, allow_zero: bool = False) -> np.ndarray: |
There was a problem hiding this comment.
Just style, but if this method is supposed to be called from outside the class (e.g. footprint_cutout.py) then I think it shouldn't be private (remove leading underscore).
I'm also curious about when we'd want allow_zero to be True.
There was a problem hiding this comment.
We use it here:
astrocut/astrocut/footprint_cutout.py
Line 238 in 4972170
I don't foresee that parameter having much use, but it is needed in this particular case to avoid an error.
There was a problem hiding this comment.
That same function is used in the classes that create cutouts, and an error should be thrown if the cutout size is zero in those cases.
There was a problem hiding this comment.
Renamed _parse_size_input to parse_size_input in last commit!
| # Cutout size of 0 should do a point match | ||
| point_results = ra_dec_crossmatch(all_ffis, coordinates, 0, 21) | ||
| len_point_results = len(point_results) | ||
| assert isinstance(point_results, Table) |
There was a problem hiding this comment.
I think it would be good to have an assertion that the crossmatch was actually a point search here.
if you end up splitting up the crossmatch code into two separate function, it should be pretty easy to apply a mock/patch to those functions to assert that the crossmatch_point func was called and not the other. maybe there are other ways to do it (does the returned table include any info we could check?)
astrocut/footprint_cutout.py
Outdated
|
|
||
|
|
||
| def ra_dec_crossmatch(all_ffis: Table, coordinates: SkyCoord, cutout_size, arcsec_per_px: int = 21) -> Table: | ||
| def _crossmatch_point(ra: SkyCoord, dec: SkyCoord, all_ffis: Table) -> List[int]: |
There was a problem hiding this comment.
return type annotation -- should it be np.ndarray?
There was a problem hiding this comment.
Ah, yes it should. Fixed!
Added support in
ra_dec_crossmatchfor a cutout size of zero, enabling single-point matching to FFIs that contain the specified coordinates. This single-point matching is significantly faster than computing an intersection between polygons.If only one axis of the cutout size is equal to zero, I add a negligible amount in that dimension and treat the cutout as a very thin rectangle.