Warn on fid light in acq search box #50
Conversation
| # Warn if there is a fid light in the search box | ||
| if ($type =~ /BOT|GUI|ACQ/){ | ||
| for my $fpos (@fid_positions){ | ||
| if (($fpos->{y} > ($yag - $halfw)) |
There was a problem hiding this comment.
I think you can do this with if (abs($fpos->{y} - $yag) < $halfw and abs($fpos-{z} - $zag) < $halfw) {
|
Good for 11.0 with the minor code update. Also, please add a comment giving the origin of the issue being checked (obsid 14577) along with a link to the discussion (yay google groups!): With all these corner cases and special rules, it is not a bad idea to have some details in the code comments. |
|
By "code comments" do you mean commit messages? I wasn't sure what was fine just being captured here in the PR. |
|
Could also be a comment that refers back to the PR. Imagine me in 3 years looking at a test and trying to understand what's going on. Getting back to the context is very useful, and can be done without too much clutter. So something like: |
|
Whereas I'm hoping that github will still exist and you'll be able to use |
| @@ -1135,6 +1138,21 @@ sub check_star_catalog { | |||
| } | |||
| } | |||
|
|
|||
| # Check for situation that occurred for obsid 14577 with a fid light | |||
| # inside the search box (PR #50). | |||
| if ($type =~ /BOT|GUI|ACQ/){ | |||
There was a problem hiding this comment.
I suppose this should just be /BOT|ACQ/ since the GUI-only case is captured in the fid spoiler check (ACA-024)
The JUL2913A week is sufficient to verify the fid-in-search-box check for PR #50.
Warn on fid light in acq search box
This just happened, and PR #50 was quite useful!! |
And detailed code comment with the provenance of the change was useful as well. |
|
Hah. If you are in fids we've also got issue #243 at some point (so fids need a bit more review in warnings and severity for 13.1). |
|
BTW, I don't remember why this is yellow (not orange) or an ACQ-only star. |
|
Um.. That is issue #243 (aka Jean suggests that ACQ-only should be orange not yellow). |
Warn on fid light in acq search box to avoid obsid 14577 issue. This should be a "yellow" warning for an acq-only star, red for a acq/guide star.