Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
ENH: Add the ability to have a separate title for each subplot when plotting #14753
Conversation
bmagnusson
added some commits
Nov 26, 2016
codecov-io
commented
Nov 27, 2016
•
Current coverage is 85.26% (diff: 0.00%)@@ master #14753 diff @@
==========================================
Files 144 144
Lines 50968 50977 +9
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 43466 43467 +1
- Misses 7502 7510 +8
Partials 0 0
|
| @@ -1217,7 +1217,11 @@ def _adorn_subplots(self): | ||
| if self.title: | ||
| if self.subplots: | ||
| - self.fig.suptitle(self.title) | ||
| + if type(self.title) == list: |
sinhrks
Nov 27, 2016
Member
pls use is_list_like, and check the input length are correct. Note that all the axes may not require title when you specify layout
bmagnusson
Nov 28, 2016
Contributor
Ok I'll use is_list_like. I purposely did not check the lengths as a feature. As it's written, if you only provide a list of two strings, but the plot has 3 suplots, the first two subplots will have a title and the third will be left without one.
|
Thx for the PR. can u add tests and release note? Also, pls link the original issue if we have. |
sinhrks
added API Design Visualization
labels
Nov 27, 2016
bmagnusson
added some commits
Nov 28, 2016
|
I did my best at adding a test and release note. I didn't see an original issue that this addresses. It's just something I have on my local that I wanted to contribute. |
| + self.assertEqual([p.title._text for p in plot], title) | ||
| + | ||
| + # Case len(title) > len(df) | ||
| + plot = df.plot(subplots=True, title=title + ['Ignore me!']) |
sinhrks
Nov 28, 2016
Member
I think we should raise if length of passed list and number of axes are different.
bmagnusson
Nov 29, 2016
Contributor
Sounds good I added exceptions and pushed. Is ValueError appropriate here?
| + for (ax, title) in zip(self.axes, self.title): | ||
| + ax.set_title(title) | ||
| + else: | ||
| + self.fig.suptitle(self.title) | ||
| else: | ||
| self.axes[0].set_title(self.title) |
|
Is there anything else I need to do for this change to get pulled in? |
| @@ -1217,8 +1217,22 @@ def _adorn_subplots(self): | ||
| if self.title: | ||
| if self.subplots: | ||
| - self.fig.suptitle(self.title) | ||
| + if is_list_like(self.title): | ||
| + if len(self.title) != len(self.axes): |
sinhrks
Dec 6, 2016
Member
pls use self.nseries. Number of columns and axes and can differ when layout is specified.
Also add test case with 3 numeric columns df with df.plot(subplots=True, layout=(2, 2))
|
@TomAugspurger pls have a look when you have a time. |
| @@ -40,7 +40,7 @@ Other enhancements | ||
| ^^^^^^^^^^^^^^^^^^ | ||
| - ``pd.read_excel`` now preserves sheet order when using ``sheetname=None`` (:issue:`9930`) | ||
| - | ||
| +- ``pd.DataFrame.plot`` now prints a title above each subplot if ``suplots=True`` and ``title`` is a list of strings |
| - self.fig.suptitle(self.title) | ||
| + if is_list_like(self.title): | ||
| + if len(self.title) != len(self.axes): | ||
| + msg = 'The length of `title` must equal the number ' \ |
TomAugspurger
Dec 6, 2016
Contributor
It'd be nice to report the length of title and the expected number of columns in the error message.
bmagnusson
added some commits
Dec 7, 2016
| + | ||
| + # Case len(title) == len(df) | ||
| + plot = df.plot(subplots=True, title=title) | ||
| + self.assertEqual([p.title._text for p in plot], title) |
TomAugspurger
Dec 7, 2016
Contributor
Sorry, I missed this earlier, but can you use p.get_title() instead of the private matplotlib ._text method. Same thing down on line 300, ax.get_title().
|
Great thanks. +1 for merge when travis passes. Just ping us if you notice that it's green before we do |
|
All green :) |
| + msg = 'The length of `title` must equal the number ' \ | ||
| + 'of columns if using `title` of type `list` ' \ | ||
| + 'and `subplots=True`.\n' \ | ||
| + 'length of title = {}\n' \ |
jorisvandenbossche
Dec 7, 2016
Owner
Can you use parentheses around the full string instead of \ for the line continuation? (for consistency within the project)
| else: | ||
| + if is_list_like(self.title): | ||
| + msg = 'Using `title` of type `list` is not supported ' \ | ||
| + 'unless `subplots=True` is passed' |
| - title : string | ||
| - Title to use for the plot | ||
| + title : string or list | ||
| + If a string is passed, print the string at the top of the figure. If a |
jorisvandenbossche
Dec 7, 2016
Owner
Can you leave the starting sentence "Title to use for the plot" ?
| - Title to use for the plot | ||
| + title : string or list | ||
| + If a string is passed, print the string at the top of the figure. If a | ||
| + list is passed and subplots is True, print each item in the |
jorisvandenbossche
Dec 7, 2016
Owner
single backticks (`) around 'subplots' (to make it clear that it is a keyword)
|
Huh. I assume there is something funny going on since I didn't change any actual code. Is there a way to restart the checks? |
bmagnusson
added some commits
Dec 8, 2016
|
Ah! I figured it out. I had a linting error (I'll start doing lint before committing oops!) |
|
All green now! Anything else to change? If not, thanks for all the help everyone! I had fun. This was my first pull request but it certainly won't be the last :). |
jorisvandenbossche
added this to the
0.20.0
milestone
Dec 9, 2016
jorisvandenbossche
merged commit 5f057cb
into pandas-dev:master
Dec 9, 2016
|
@bmagnusson Thanks! Looking forward to more PRs :-) |
yarikoptic
added a commit
to neurodebian/pandas
that referenced
this pull request
Dec 12, 2016
|
|
yarikoptic |
6c87601
|
yarikoptic
added a commit
to neurodebian/pandas
that referenced
this pull request
Dec 12, 2016
|
|
yarikoptic |
dd7e977
|
ischurov
added a commit
to ischurov/pandas
that referenced
this pull request
Dec 19, 2016
|
bmagnusson + ischurov |
bd59882
|
bmagnusson commentedNov 26, 2016
•
edited
git diff upstream/master | flake8 --diff