New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue/130/grb filters #131
Conversation
Codecov ReportBase: 65.95% // Head: 67.39% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #131 +/- ##
==========================================
+ Coverage 65.95% 67.39% +1.44%
==========================================
Files 13 14 +1
Lines 420 457 +37
==========================================
+ Hits 277 308 +31
- Misses 143 149 +6
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Shall I do the review now finally, or shall I wait? |
Wait until the end of the test-suite and if all is green, you can merge the pull request |
You can do the review now, and I will merge if you say everything is ok. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @FusRoman ! It is ok for me, but I left a few comments before merging.
fink_filters/__init__.py
Outdated
@@ -12,4 +12,5 @@ | |||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
__version__ = "3.7" | |||
|
|||
__version__ = "3.8.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The versioning is done using x.y -- you should put 3.8
@@ -0,0 +1,186 @@ | |||
# Copyright 2019-2022 AstroLab Software |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2023
25 | ||
""" | ||
f_bogus = realbogus_score >= 0.5 | ||
f_class = fink_class.isin(["SN candidate", "Unknown", "Ambiguous"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here there is a potential problem. If SIMBAD is down, you will reject all alerts.
Alternatively, you can add a guard against downtime of SIMBAD. You could have another filter which is:
f_fail = fink_class.apply(lambda x: x.startswith('Fail'))
f_bronze = f_bogus & (f_class | f_fail)
But note that in this case, if SIMBAD is down, only f_bogus
will apply.
IMPORTANT: Please create an issue first before opening a Pull Request.
Linked to issue(s): Closes put link
What changes were proposed in this pull request?
How is the issue this PR is referenced against solved with this PR?
How was this patch tested?