-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: MaskedArray._quantile match non-nullable behavior #46282
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
Changes from 2 commits
de59af4
c4cd9c1
dcc33f2
8a6b82a
f80ddc8
fe2d38e
348a08a
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 | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -181,9 +181,20 @@ def test_highlight_between_inclusive(styler, inclusive, expected): | |||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
def test_highlight_quantile(styler, kwargs): | ||||||||||||||||||||||||||||
expected = { | ||||||||||||||||||||||||||||
(0, 1): [("background-color", "yellow")], | ||||||||||||||||||||||||||||
(2, 0): [("background-color", "yellow")], | ||||||||||||||||||||||||||||
(2, 1): [("background-color", "yellow")], | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
if styler.data.dtypes["B"] != "Int64": | ||||||||||||||||||||||||||||
expected.pop((0, 1)) | ||||||||||||||||||||||||||||
else: | ||||||||||||||||||||||||||||
if kwargs.get("axis", -1) is None: | ||||||||||||||||||||||||||||
expected.pop((0, 1)) | ||||||||||||||||||||||||||||
elif kwargs.get("q_left", -1) == 0: | ||||||||||||||||||||||||||||
expected.pop((0, 1)) | ||||||||||||||||||||||||||||
elif "subset" in kwargs: | ||||||||||||||||||||||||||||
expected.pop((0, 1)) | ||||||||||||||||||||||||||||
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.
Suggested change
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. this fixes the test, although I would not intuitively expect the results noted in the comment. 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. [[ the alternative fix is to change the pytest fixture column values from {1, pd.NA, 2} to {1, pd.Na, 3}, since then the value 1 is not caught within the [0.5, 1] quantile where pd.NA = 0, and conveniently all other tests in the module still pass ]] 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. Do you think the trouble is in the quantile behavior or in the Styler? 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. OK, further investigation shows that this is a rounding issue when preserving the dtype in the quantile function. With >>> df
A B
0 0 1
1 <NA> <NA>
2 10 2 Then calling A B
0.5 5.0 1.5
1.0 10.0 2.0
dtypes=Float64 Whereas under this PR using A B
0.5 5 1
1.0 10 2
dtypes=Int64 The value of 1 is included in the second, probably due to calling Prior to this PR the dtype of column B was upcast from "Int64" to "float64" and the leftmost quantile was calculated as 1.5. Since the interpolation style is linear and that is specifically documented to be fraction, I think it would be worth expanding the docs how dtypes are preserved and can affect calculations, or considering upcasting to "Float64". It is possible to fix all tests in Styler by changing the fixture's column B to have 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. thanks @attack68 I think I've got a handle on whats going wrong, and it tentatively looks like its in the quantile code. ill try the fix i have in mind and let you know if that doesnt work out |
||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
result = styler.highlight_quantile(**kwargs)._compute().ctx | ||||||||||||||||||||||||||||
assert result == expected | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
|
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.