Skip to content
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

add filter criteria #831

Merged
merged 4 commits into from
May 3, 2024
Merged

add filter criteria #831

merged 4 commits into from
May 3, 2024

Conversation

anaismoller
Copy link
Member

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?

@JulienPeloton
Copy link
Member

Thanks @anaismoller for the contribution! Looking at the code, the only difference with respect to bin/active_learning_loop.py is the addition of a new condition c0 = df["al_snia_vs_nonia"] > 0.5?

If so, no need to duplicate the entire file. Code duplication is typically to be avoided as it makes the code more difficult to maintain, and more prone to error on the long term. I pushed a suggestion.

However, if you want to send candidates on a slack channel outside Fink, I will need to extend the code behind msg_handler_slack which supposes Fink channels only for the moment.

@JulienPeloton
Copy link
Member

JulienPeloton commented May 2, 2024

removed

@anaismoller
Copy link
Member Author

anaismoller commented May 3, 2024

Thanks @JulienPeloton

Indeed, it is better without duplication.

Can we send this high_probability SNe Ia to Fink slack channel bot_al_loop_highprob ? thanks!

Copy link

sonarcloud bot commented May 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@JulienPeloton JulienPeloton merged commit 8aab155 into master May 3, 2024
19 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants