Fix for Issue # 1643 #1645
Fix for Issue # 1643 #1645
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## develop #1645 +/- ##
===========================================
+ Coverage 40.98% 41.34% +0.35%
===========================================
Files 189 189
Lines 43055 42685 -370
===========================================
Hits 17646 17646
+ Misses 25409 25039 -370
|
There was a problem hiding this comment.
Looks good @debora-pe but I'm not following the control flow that well. I've asked @rcooke-ast to also review since I think he was the original author of this code. I think this probably requires that the dev-suite be run, as there could be unforeseen consequences of modifying such a core algorithm when fits to slits start failing and things are masked.
pypeit/core/wavecal/autoid.py
Outdated
| @@ -1772,11 +1774,10 @@ def run_brute(self, min_nlines=10): | |||
|
|
|||
| # Now that all slits have been inspected, cross match to generate a | |||
| # list of all lines in every slit, and refit all spectra | |||
| if self._nslit > 1: | |||
| obad_slits = self.cross_match(good_fit, self._det_weak) if self._ok_mask.size > 1 else np.array([]) | |||
| if obad_slits.size > 1: | |||
There was a problem hiding this comment.
Should this be >=? The previous if self._nslit > 1: was checking if this was multislit vs longslit, I think, but this new check is looking explicitly for bad slits.
| msgs.info('Checking wavelength solution by cross-correlating with all slits') | ||
|
|
||
| msgs.info('Cross-correlation iteration #1') |
There was a problem hiding this comment.
There are several options here, but I think the two information messages should be put above obad_slits = self.cross_match(...) blah. Alternatively, the second info message could be removed and change cntr=1. Otherwise, there appears to be printout that says iteration 1 and then immediately iteration 2
There was a problem hiding this comment.
@rcooke-ast thank you so much for your comments. They made me realize that the flow here was not completely correct, so I fixed it. Please take a look if you can.
I'm running the Dev Suite right now.
|
@jhennawi thank you for reviewing this. I didn't know if you wanted me to modify the pypeit file in the Dev Suite that triggered this problem. I made the change here: pypeit/PypeIt-development-suite#279 |
|
Tests pass, except for a few unit tests failures that are not related to this PR and that PR #1628 (Dev Suite PR #272) has fixed. The skipped tests were due to not having specutils installed, so I re run them and all good. |
|
Merging this |
As titled. Issue #1643
The problem was not in the
slitspatnumimplementation but in dealing with masked slits in HolyGrail