-
Notifications
You must be signed in to change notification settings - Fork 50
Add EqualizedOddsImprovement metric
#775
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #775 +/- ##
==========================================
+ Coverage 95.64% 95.73% +0.09%
==========================================
Files 115 117 +2
Lines 4590 4736 +146
==========================================
+ Hits 4390 4534 +144
- Misses 200 202 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
85aadc3 to
6e3fbca
Compare
6e3fbca to
482328d
Compare
R-Palazzo
left a comment
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.
This is looking good!
Could you add a integration where the sensitive_column_value is np.nan
| for is_sensitive_group in [True, False]: | ||
| group_predictions = prediction_binary[sensitive_binary == is_sensitive_group] | ||
| group_name = 'sensitive' if is_sensitive_group else 'non-sensitive' | ||
|
|
||
| if len(group_predictions) == 0: | ||
| raise ValueError(f'No data found for {group_name} group.') | ||
|
|
||
| positive_count = group_predictions.sum() | ||
| negative_count = len(group_predictions) - positive_count | ||
|
|
||
| if positive_count < 5 or negative_count < 5: | ||
| raise ValueError( | ||
| f'Insufficient data for {group_name} group: {positive_count} positive, ' | ||
| f'{negative_count} negative examples (need ≥5 each).' |
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.
Do we need a for loop here since we are counting both positive and negative?
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.
Yes. There are two groups (1) data that matches the sensitive value and (2) data that doesn't. For both cases we need at least 5 True target values and 5 False target values.
| data[sensitive_column_name] = ( | ||
| data[sensitive_column_name] == sensitive_column_value | ||
| ).astype(int) |
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.
Does it work if the sensitive_column_value is np.nan?
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.
A good question. If the column is categorical the nans are just another category, since the column dtype is object, so the user should pass 'nan', 'None', or whatever the string representation they are using for missing values is.
It won't reach the line of code you are referring if the user passes np.nan, since the data validation before this will complain np.nan is not present in the data. I added a test showing this.
If the data is numerical it would indeed crash. I added new logic to handle it both in these lines of code and the validation.
R-Palazzo
left a comment
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.
LGTM!
frances-h
left a comment
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.
Nice work!
CU-86b5ayqd6, Resolve #772