-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: Implement xlabel and ylabel options in Series.plot and DataFrame.plot #34223
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 17 commits
7e461a1
1314059
8bcb313
24c3ede
dea38f2
cd9e7ac
e5e912b
045a76f
e69c080
bc7bad4
4ae4f17
0d32836
4d7216b
cc1b6d6
dbf8f1a
3fb1faa
f8ad37a
3f34099
4246f6e
408d08f
54824ab
8a24a40
6bb571a
66cf42f
7e2a7c9
31c8ccf
9d1d942
5335de8
2a8406d
8f12150
e1ade53
26bfa6e
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 |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import re | ||
from typing import Optional | ||
from typing import List, Optional | ||
import warnings | ||
|
||
import numpy as np | ||
|
@@ -95,6 +95,8 @@ def __init__( | |
ylim=None, | ||
xticks=None, | ||
yticks=None, | ||
xlabel: Optional[str] = None, | ||
ylabel: Optional[str] = None, | ||
sort_columns=False, | ||
fontsize=None, | ||
secondary_y=False, | ||
|
@@ -136,6 +138,8 @@ def __init__( | |
self.ylim = ylim | ||
self.title = title | ||
self.use_index = use_index | ||
self.xlabel = xlabel | ||
self.ylabel = ylabel | ||
|
||
self.fontsize = fontsize | ||
|
||
|
@@ -153,8 +157,8 @@ def __init__( | |
|
||
self.grid = grid | ||
self.legend = legend | ||
self.legend_handles = [] | ||
self.legend_labels = [] | ||
self.legend_handles: List = [] | ||
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. What error does this solve? 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 solves mypy complaint, exactly the same as in #28373 |
||
self.legend_labels: List = [] | ||
|
||
for attr in self._pop_attributes: | ||
value = kwds.pop(attr, self._attr_defaults.get(attr, None)) | ||
|
@@ -480,6 +484,11 @@ def _adorn_subplots(self): | |
if self.xlim is not None: | ||
ax.set_xlim(self.xlim) | ||
|
||
# GH9093, currently Pandas does not show ylabel, so if users provide | ||
# ylabel will set it as ylabel in the plot. | ||
if self.ylabel is not None: | ||
ax.set_ylabel(self.ylabel) | ||
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. Is this consistent with May be I'm wrong, but feels like Would be probably good to add a test in the parametrization to make sure this works as expected. 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. good call! i think matplotlib's But i think it might be indeed nicer to be consistent with |
||
|
||
ax.grid(self.grid) | ||
|
||
if self.title: | ||
|
@@ -666,6 +675,10 @@ def _get_index_name(self): | |
if name is not None: | ||
name = pprint_thing(name) | ||
|
||
# GH 9093, override the default xlabel if xlabel is provided. | ||
if self.xlabel is not None: | ||
name = pprint_thing(self.xlabel) | ||
|
||
return name | ||
|
||
@classmethod | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3350,6 +3350,62 @@ def test_colors_of_columns_with_same_name(self): | |
for legend, line in zip(result.get_legend().legendHandles, result.lines): | ||
assert legend.get_color() == line.get_color() | ||
|
||
@pytest.mark.parametrize( | ||
"index_name, old_xlabel, new_xlabel, old_ylabel, new_ylabel", | ||
[ | ||
(None, "", "new_x", "", ""), | ||
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. Not a huge issue but instead of parametrizing xlabel / ylabel as individual arguments can you not just pass one set of labels and maybe have another parametrization on 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. you are right, actually we can also remove the need of Pls let me know your thoughts on the simplified tests! thanks! @WillAyd |
||
("old_x", "old_x", "new_x", "", ""), | ||
(None, "", "", "", ""), | ||
(None, "", "new_x", "", "new_y"), | ||
("old_x", "old_x", "new_x", "", "new_y"), | ||
], | ||
) | ||
@pytest.mark.parametrize("kind", ["line", "area", "bar"]) | ||
def test_xlabel_ylabel_dataframe_single_plot( | ||
self, kind, index_name, old_xlabel, old_ylabel, new_xlabel, new_ylabel | ||
): | ||
# GH 9093 | ||
df = pd.DataFrame([[1, 2], [2, 5]], columns=["Type A", "Type B"]) | ||
df.index.name = index_name | ||
|
||
# default is the ylabel is not shown and xlabel is index name | ||
ax = df.plot(kind=kind) | ||
assert ax.get_ylabel() == old_ylabel | ||
assert ax.get_xlabel() == old_xlabel | ||
|
||
# old xlabel will be overriden and assigned ylabel will be used as ylabel | ||
ax = df.plot(kind=kind, ylabel=new_ylabel, xlabel=new_xlabel) | ||
assert ax.get_ylabel() == new_ylabel | ||
assert ax.get_xlabel() == new_xlabel | ||
|
||
@pytest.mark.parametrize( | ||
"index_name, old_xlabel, new_xlabel, old_ylabel, new_ylabel", | ||
[ | ||
(None, "", "new_x", "", ""), | ||
("old_x", "old_x", "new_x", "", ""), | ||
(None, "", "", "", ""), | ||
(None, "", "new_x", "", "new_y"), | ||
("old_x", "old_x", "new_x", "", "new_y"), | ||
], | ||
) | ||
@pytest.mark.parametrize("kind", ["line", "area", "bar"]) | ||
def test_xlabel_ylabel_dataframe_subplots( | ||
self, kind, index_name, old_xlabel, old_ylabel, new_xlabel, new_ylabel | ||
): | ||
# GH 9093 | ||
df = pd.DataFrame([[1, 2], [2, 5]], columns=["Type A", "Type B"]) | ||
df.index.name = index_name | ||
|
||
# default is the ylabel is not shown and xlabel is index name | ||
axes = df.plot(kind=kind, subplots=True) | ||
assert all(ax.get_ylabel() == old_ylabel for ax in axes) | ||
assert all(ax.get_xlabel() == old_xlabel for ax in axes) | ||
|
||
# old xlabel will be overriden and assigned ylabel will be used as ylabel | ||
axes = df.plot(kind=kind, ylabel=new_ylabel, xlabel=new_xlabel, subplots=True) | ||
assert all(ax.get_ylabel() == new_ylabel for ax in axes) | ||
assert all(ax.get_xlabel() == new_xlabel for ax in axes) | ||
|
||
|
||
def _generate_4_axes_via_gridspec(): | ||
import matplotlib.pyplot as plt | ||
|
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.
Does this have to be string? Should work with e.g. int / float
Anyway, I tried running this and it looks good! Nice tests
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.
yeah, I think it should be string? this is not calling any column of dataframe, but explicitly defined by users who want to give labels for x/y axis. I am not very sure about if users would name it using int/float, quite rare case though
I will later check to see if matplotlib allows to do so or not, if so, maybe we could support too! thanks!
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.
I tried running it passing an int, and it worked fine - but yeah, typical case would be to label with a
str
, so it's fine as is, sorry for the noise :)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.
I will change to
Label
then and add a test case, so that we also support numbers!Thanks for the feedback!