-
Notifications
You must be signed in to change notification settings - Fork 237
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 Evaluate
usage for scikit-learn
#368
Conversation
The documentation is not available anymore as the PR was closed or merged. |
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.
Hi @awinml, this looks great - thanks for adding! Left a few minor comments.
Evaluate
usage in scikit-learn
Evaluate
usage in scikit-learn
Evaluate
usage in scikit-learn
Evaluate
usage for scikit-learn
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.
Two nits and then we can merge :) Something is a bit off with the PR since it also shows changes to some previous changes. Looking at the history it seems like there have been multiple branches starting in different points of the git history that got merged for this PR. I think it's just git that's confused but and when we merge it should be ok.
I rebased the merge with master by mistake. So it pulled in all those commits. |
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.
Just one minor comment left then we can merge. Let's add this and wait for the CI to get green to merge.
I have added the actual value for the accuracy. |
@lvwerra I reverted the incorrect commits and merged with main. The CI tests fail on unrelated tests. Do you have any ideas on how to fix it? |
You should not revert the changes. Ideally you just merge |
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
c7afba9
to
da5dd2c
Compare
@lvwerra I am extremely sorry for the messy commit history. I did a complete rebase and cherry picked all the correct commits. |
No worries, that's why we squash :) Thanks for your contribution! |
Reference Issues/PRs
Fixes #326
What does this implement/fix?
Adds an example demonstrating the integration of
evaluate
with the scikit-learn framework.Note:
Instead of creating a section in
metrics_integrations.mdx
, I have decided to createsklearn_integrations.mdx
so that it is consistent with the previous section onTransformers
.cc @lvwerra @osanseviero