Skip to content
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

BUGFIX: line_kws.color is of no effect in distribution plot #3691

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

Conversation

youqingxiaozhua
Copy link

In the distribution plot (such as seaborn.histplot), both color and line_kws['color'] parameters are allowed to be passed to control the colors of bars and lines, respectively. However, current codes directly take the line color from the bar color while ignoring the line_kws['color']. This pull request fixes this by looking at line_kws['color'] first, and adopting bar color only if the line color is not manually set.

@mwaskom
Copy link
Owner

mwaskom commented May 10, 2024

Can you provide a reproducible example of the issue? I am not sure this is a bug.

@youqingxiaozhua
Copy link
Author

Can you provide a reproducible example of the issue? I am not sure this is a bug.

Please see this notebook: https://colab.research.google.com/drive/1Ltc4lJzZjJXGyxdtlSOwtRGry2oMDWT6?usp=sharing

@@ -647,7 +647,8 @@ def plot_univariate_histogram(
line_args = density, support
sticky_x, sticky_y = (0, np.inf), None

line_kws["color"] = to_rgba(sub_color, 1)
line_color = line_kws.get('color', sub_color)
line_kws["color"] = to_rgba(line_color, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment breaks hue KDE line color (when not using line_kws) - it makes all KDE lines use the same color.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. The latest comment fixes this by reading the custom color value out of the hue for loop.

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.

3 participants