-
Notifications
You must be signed in to change notification settings - Fork 164
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
added psi calculation to categorical columns #1027
Changes from 4 commits
4792144
026b2ef
86aa39a
108d8d8
d3d52f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -726,10 +726,17 @@ def test_categorical_diff(self): | |
"df": 2, | ||
"p-value": 0.3099238764710244, | ||
}, | ||
"psi": 0, | ||
}, | ||
} | ||
|
||
self.assertDictEqual(expected_diff, profile.diff(profile2)) | ||
with self.assertWarnsRegex( | ||
RuntimeWarning, | ||
"psi was not calculated due to the differences in categories " | ||
"of the profiles. Differences:\n{'maybe'}\n" | ||
"defaulting psi value to 0...", | ||
): | ||
test_profile_diff = profile.diff(profile2) | ||
self.assertDictEqual(expected_diff, test_profile_diff) | ||
|
||
# Test with one categorical column matching | ||
df_not_categorical = pd.Series( | ||
|
@@ -756,6 +763,38 @@ def test_categorical_diff(self): | |
} | ||
self.assertDictEqual(expected_diff, profile.diff(profile2)) | ||
|
||
# Test diff with psi enabled | ||
df_categorical = pd.Series(["y", "y", "y", "y", "n", "n", "n", "maybe"]) | ||
profile = CategoricalColumn(df_categorical.name) | ||
profile.update(df_categorical) | ||
|
||
df_categorical = pd.Series(["y", "maybe", "y", "y", "n", "n", "maybe"]) | ||
profile2 = CategoricalColumn(df_categorical.name) | ||
profile2.update(df_categorical) | ||
|
||
# chi2-statistic = sum((observed-expected)^2/expected for each category in each column) | ||
# df = categories - 1 | ||
# psi = (% of records based on Sample (A) - % of records Sample (B)) * ln(A/ B) | ||
# p-value found through using chi2 CDF | ||
expected_diff = { | ||
"categorical": "unchanged", | ||
"statistics": { | ||
"unique_count": "unchanged", | ||
"unique_ratio": -0.05357142857142855, | ||
"chi2-test": { | ||
"chi2-statistic": 0.6122448979591839, | ||
"df": 2, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. outside scope of this PR: Does df stand for dataframe here? Looks like it's an int. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No its supposed to an int, its stands for degrees of freedom, but I agree that it is not a great choice of name |
||
"p-value": 0.7362964551863367, | ||
}, | ||
"categories": "unchanged", | ||
"gini_impurity": -0.059311224489795866, | ||
"unalikeability": -0.08333333333333326, | ||
"psi": 0.16814961527477595, | ||
"categorical_count": {"y": 1, "n": 1, "maybe": -1}, | ||
}, | ||
} | ||
self.assertDictEqual(expected_diff, profile.diff(profile2)) | ||
|
||
def test_unalikeability(self): | ||
df_categorical = pd.Series(["a", "a"]) | ||
profile = CategoricalColumn(df_categorical.name) | ||
|
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.
Maybe if we hit this we should just either 1) have PSI to None or 2) not include in the difference ducts
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.
I agree with this if PSI cant be calculated it shouldnt default to zero
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.
Done