Updates for make_derived_metric#649
Merged
riedgar-ms merged 12 commits intofairlearn:masterfrom Dec 11, 2020
riedgar-ms:riedgar-ms/tweak-make-derived-metric
Merged
Updates for make_derived_metric#649riedgar-ms merged 12 commits intofairlearn:masterfrom riedgar-ms:riedgar-ms/tweak-make-derived-metric
riedgar-ms merged 12 commits intofairlearn:masterfrom
riedgar-ms:riedgar-ms/tweak-make-derived-metric
Conversation
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
riedgar-ms
commented
Dec 3, 2020
…make-derived-metric
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Member
|
It was easier for me to implement the changes in a PR @riedgar-ms , which you can see here: https://github.com/riedgar-ms/fairlearn/pull/1 |
Convert to use an `sklearn` pipeline for the data processing. Signed-off-by: adrinjalali <adrin.jalali@gmail.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
Signed-off-by: Richard Edgar <riedgar@microsoft.com>
riedgar-ms
commented
Dec 9, 2020
adrinjalali
approved these changes
Dec 10, 2020
Member
adrinjalali
left a comment
There was a problem hiding this comment.
Thanks @riedgar-ms , LGTM.
mesameki
approved these changes
Dec 10, 2020
Member
|
woohoo, two approvals, merging :) |
Member
|
oh, I can't lol |
Member
Author
That's for our ToDo list - make sure you have the right permissions @adrinjalali (and @hildeweerts too). Although in this case, since there's a DCO mess up (I think I did this when I squashed the PR you made against my fork, rather than just merging it), it might take owner permissions. At least until we kill off DCO.... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I had overlooked some comments on the previous PR for
make_derived_metric(). This is to address those points.The largest change is to the data preprocessing, which has been adjusted to avoid the problem of Data Leakage between the training and test sets (credit to @adrinjalali for the actual
Pipelinecode). This fix has been ported to the other metrics example.