-
Notifications
You must be signed in to change notification settings - Fork 72
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 scoring
parameter for SplineCV
#380
Conversation
💖 Thank you for opening your first pull request in this repository! 💖 A few things to keep in mind:
⭐ No matter what, we are really grateful that you put in the effort to do this! ⭐ |
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.
Thank you for adding this! Made a few minor suggestions and a question at the end about the test. Otherwise, this looks very good to me. Just waiting for the CI to finish running tests and checks to see if there is anything else that needs addressing.
Co-authored-by: Leonardo Uieda <leouieda@gmail.com>
Co-authored-by: Leonardo Uieda <leouieda@gmail.com>
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.
Looks great! Merging this in 🚀
🎉 Congrats on merging your first pull request and welcome to the team! 🎉 If you would like to be added as a author on the Zenodo archive of the next release, add your full name, affiliation, and ORCID (optional) to the We hope that this was a good experience for you. Let us know if there is any way that the contributing process could be improved. |
@JamesSample please see above ☝🏾 about adding yourself as an author 🙂 Thanks for taking the time to report and implement this! |
I've added
scoring
as an additional parameter inSplineCV
to control the scoring metric used during cross-validation. I've also added a simple test totest_spline.py
, which checks that results fromSplineCV
using the new scoring parameter match those from standardSpline
combined withcross_val_score
. I'm not sure I've put this test in the right place - please let me know if it should be elsewhere.Relevant issues/PRs:
(Hopefully) fixes #379 (see also the discussion here).