Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
Loading
from

Conversation

AndPuQing
Copy link

@AndPuQing AndPuQing commented Nov 11, 2024

Overview

Closes #3895

Description of the changes proposed in this pull request:

Add tolist to ensure that the builtin Python type

Checklist

  • All pre-commit checks pass.
  • Unit tests added (if fixing a bug or adding a new feature)

@CloseChoice
Copy link
Collaborator

CloseChoice commented Dec 2, 2024

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?
I would also appreciate a test, at best using the example from the issue.

@AndPuQing
Copy link
Author

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? I would also appreciate a test, at best using the example from the issue.


This PR fixes a bug where plot_text color appears as black.

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

Example

In 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.

Fix

The following code modification resolves the issue by converting NumPy arrays to Python lists using .tolist(), ensuring proper string representations:

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.

@AndPuQing
Copy link
Author

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():
Copy link
Collaborator

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Class impact visualization on multiclass semantic classification is all black
2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.