BUG: don't plot colorbar if c is column containing colors #34344
Conversation
pandas/plotting/_matplotlib/core.py
Outdated
| @@ -969,6 +966,16 @@ def _make_plot(self): | |||
| else: | |||
| c_values = c | |||
|
|
|||
| # plot a colorbar only if a colormap is provided or necessary | |||
There was a problem hiding this comment.
maybe remove this line and add a more descriptive comment for the next line (c_is_column_not_containing_colors)? this comment does not seem to add too much value to help others understand?
pandas/plotting/_matplotlib/core.py
Outdated
| # plot a colorbar only if a colormap is provided or necessary | ||
| from matplotlib.colors import is_color_like | ||
|
|
||
| c_is_column_not_containing_colors = c_is_column and not all( |
There was a problem hiding this comment.
maybe currently we should only consider if c_values are numeric, instead of checking if not all are color-like?
because your other PR are still under development, and seems no concrete conclusion on how we will show categorical/color-like in colorbar/legend, so now only having numeric values on colorbar seems most reasonable.
There was a problem hiding this comment.
Yes, good point - thanks for your review, @charlesdong1991 !
MarcoGorelli
changed the title
c is not a column name
pandas/plotting/_matplotlib/core.py
Outdated
| c_is_numeric_col = c_is_column and is_numeric_dtype(c_values) | ||
| cb = self.kwds.pop("colorbar", self.colormap or c_is_numeric_col) |
There was a problem hiding this comment.
emm, i meant something like this:
# plot colorbar if 1. colormap is assigned, 2.`c` is a column containing only numeric values
plot_colorbar = self.colormap or c_is_column
cb = self.kwds.pop("colorbar", is_numeric_dtype(c_values) and plot_colorbar)because even when colormap is defined, if not all values of c column are numeric, we still dont want to plot colorbar, because it doesnt make sense so far. What do you think?
There was a problem hiding this comment.
Yes, you're right, thanks! The way I'd originally written it there would still be a colorbar if the user passed something to cmap
| @@ -969,6 +967,12 @@ def _make_plot(self): | |||
| else: | |||
| c_values = c | |||
|
|
|||
| # plot colorbar if | |||
| # 1. colormap is assigned, and | |||
| # 2.`c` is a column containing only numeric values | |||
There was a problem hiding this comment.
Do we have a test case that hits if non-numeric values are supplied? If not can you add one?
There was a problem hiding this comment.
@WillAyd , yup, it's test_scatter_with_c_column_name_with_colors, where c='species', and the column species contains ["r", "r", "g", "g", "b"]
|
thanks @MarcoGorelli |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff