Repository: spacetelescope/tweakwcs

Issue:
Peak measurement of 2d histogram can find unrealistic positions for the peak
Processing the singleton 'ibbr02c5q' triggers a logical error when determining the position of the peak of the 2d histogram.  

Specifically, Line 489 of `matchutils.py` in the `_find_peak()` function can return a solution that ends up outside the range of values defined by the slice being used for the fit defined by (x1, x2), (y1,y2).  Logic needs to be added to look for this situation and trigger the failure mode to revert to simply reporting the position of the max value instead as currently happens now when the 'det <= 0' in Line 513.  The current logic does not prevent this function from returning values outside of the slice defined for the peak.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) Report
> Merging [#136](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) (7c11a9e) into [master](https://codecov.io/gh/spacetelescope/tweakwcs/commit/0056b97943e6a0649685b0d93fdf66491b60d86e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) (0056b97) will **decrease** coverage by `0.05%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136/graphs/tree.svg?width=650&height=150&src=pr&token=YjZkswRHyv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope)](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope)

```diff
@@            Coverage Diff             @@
##           master     #136      +/-   ##
==========================================
- Coverage   85.93%   85.87%   -0.06%     
==========================================
  Files          10       10              
  Lines        1827     1827              
  Branches      270      270              
==========================================
- Hits         1570     1569       -1     
- Misses        252      253       +1     
  Partials        5        5              
```


| [Impacted Files](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Coverage Δ | |
|---|---|---|
| [tweakwcs/matchutils.py](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-dHdlYWt3Y3MvbWF0Y2h1dGlscy5weQ==) | `100.00% <100.00%> (ø)` | |
| [tweakwcs/imalign.py](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope#diff-dHdlYWt3Y3MvaW1hbGlnbi5weQ==) | `99.40% <0.00%> (-0.60%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope). Last update [0056b97...7c11a9e](https://codecov.io/gh/spacetelescope/tweakwcs/pull/136?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope).


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index ebe60f5..2dde896 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -8,9 +8,12 @@ Release Notes
    ==================
 
 
-0.7.2 (06-May-2021)
+0.7.2 (07-May-2021)
 ===================
 
+- Fixed a bug in ``matchutils._find_peak()`` due to which it could return
+  coordinates of the peak that were outside of the image. [#136]
+
 - Fixed a bug in how re-projection was computed when ``center`` of the
   transformations was provided. [#135]
 
diff --git a/tweakwcs/matchutils.py b/tweakwcs/matchutils.py
index 9fd05e9..6def57b 100644
--- a/tweakwcs/matchutils.py
+++ b/tweakwcs/matchutils.py
@@ -518,12 +518,12 @@ def _find_peak(data, peak_fit_box=5, mask=None):
     ym = (c10 * c11 - 2.0 * c01 * c20) / det
 
     if 0.0 <= xm <= (nx - 1.0) and 0.0 <= ym <= (ny - 1.0):
-        coord = (xm, ym)
         fit_status = 'SUCCESS'
-
     else:
         xm = 0.0 if xm < 0.0 else min(xm, nx - 1.0)
         ym = 0.0 if ym < 0.0 else min(ym, ny - 1.0)
         fit_status = 'WARNING:EDGE'
 
+    coord = (xm, ym)
+
     return coord, fit_status, np.s_[y1:y2, x1:x2]


Test patch:
diff --git a/tweakwcs/tests/test_matchutils.py b/tweakwcs/tests/test_matchutils.py
index a390812..59d6dd3 100644
--- a/tweakwcs/tests/test_matchutils.py
+++ b/tweakwcs/tests/test_matchutils.py
@@ -189,7 +189,7 @@ def test_find_peak_fit_over_edge():
     data[:, 0] = 0.0
     coord, fit_status, fit_box = _find_peak(data, peak_fit_box=7)
     assert fit_status == 'WARNING:EDGE'
-    assert np.allclose(coord, (1, 11), rtol=0, atol=1e-6)
+    assert np.allclose(coord, (0, 11), rtol=0, atol=1e-6)
 
 
 @pytest.mark.parametrize('shift', [100, 2])