-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Refactor color tuple creation to ensure compatibility with RGB values #3911
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
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR @AndPuQing . Was there a special case where values were wrongly set to 0 that is fixed with your function? Could you please explain why this fixes issue #3895? |
This PR fixes a bug where The issue arises due to a change in the string representation of NumPy scalars introduced in NumPy 2.x. Specifically, this change affects how arrays are converted to strings. For reference, see the NumPy 2.0 release notes ExampleIn NumPy 2.0: >>> import numpy as np
>>> x = np.ones(4)
>>> tuple(x)
(np.float64(1.0), np.float64(1.0), np.float64(1.0), np.float64(1.0)) In this case, the CSS color fails to take effect because the string representations are invalid, resulting in the color being displayed as black. In NumPy 1.x: >>> import numpy as np
>>> x = np.ones(4)
>>> tuple(x)
(1.0, 1.0, 1.0, 1.0) Here, the string representations are valid, and the CSS color is applied correctly. FixThe following code modification resolves the issue by converting NumPy arrays to Python lists using def scale_color_to_255(color: tuple) -> tuple[int, int, int, float]:
color = np.array(color).tolist() if isinstance(color[0], (np.float64, np.float32)) else color
return tuple(int(c * 255) if i < 3 else c for i, c in enumerate(color)) This ensures the string representation is valid, and the colors are displayed correctly. |
And sorry, because I used black format, there are many changes in code style. |
@@ -29,3 +37,40 @@ def test_single_text_to_text(): | ||
hierarchical_values=[test_hierarchical_values], | ||
) | ||
shap.plots.text(shap_values_test) | ||
|
||
|
||
def test_color(): |
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.
Would be great if you could use pytest-mpl and our test setup for plots here. Have a look at:
(baseline plots, pytest-mpl, a test of ours)
Overview
Closes #3895
Description of the changes proposed in this pull request:
Add
tolist
to ensure that the builtin Python typeChecklist