ENH: Add the ability to have a separate title for each subplot when plotting by icanhazcodeplz-zz · Pull Request #14753 · pandas-dev/pandas (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation29 Commits15 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@icanhazcodeplz-zz

@icanhazcodeplz-zz

…each item of the list as the title of the individual subplots.

@icanhazcodeplz-zz

@icanhazcodeplz-zz

@codecov-io

Current coverage is 85.26% (diff: 0.00%)

Merging #14753 into master will decrease coverage by 0.01%

@@ master #14753 diff @@

Files 144 144
Lines 50968 50977 +9
Methods 0 0
Messages 0 0
Branches 0 0

Powered by Codecov. Last update 3ac41ab...59ab880

sinhrks

if self.title:
if self.subplots:
self.fig.suptitle(self.title)
if type(self.title) == list:

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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.

@sinhrks

Thx for the PR. can u add tests and release note? Also, pls link the original issue if we have.

@icanhazcodeplz-zz

@icanhazcodeplz-zz

@icanhazcodeplz-zz

…ots' == True and 'title' is a list

@icanhazcodeplz-zz

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.

sinhrks

self.assertEqual([p.title._text for p in plot], title)
# Case len(title) > len(df)
plot = df.plot(subplots=True, title=title + ['Ignore me!'])

Choose a reason for hiding this comment

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

I think we should raise if length of passed list and number of axes are different.

Choose a reason for hiding this comment

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

Sounds good I added exceptions and pushed. Is ValueError appropriate here?

else:
self.fig.suptitle(self.title)
else:
self.axes[0].set_title(self.title)

Choose a reason for hiding this comment

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

should raise if subplots=False and input is list-like

@icanhazcodeplz-zz

-raise ValueError if subplots=False and title is of type list

@icanhazcodeplz-zz

Is there anything else I need to do for this change to get pulled in?

sinhrks

if self.subplots:
self.fig.suptitle(self.title)
if is_list_like(self.title):
if len(self.title) != len(self.axes):

Choose a reason for hiding this comment

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

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))

Choose a reason for hiding this comment

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

Done :)

@sinhrks

TomAugspurger

- ``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

Choose a reason for hiding this comment

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

You can add this PR number in the issue tag (:issue:14753)

Choose a reason for hiding this comment

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

Done :)

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 ' \

Choose a reason for hiding this comment

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

It'd be nice to report the length of title and the expected number of columns in the error message.

Choose a reason for hiding this comment

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

Done :)

@icanhazcodeplz-zz

@icanhazcodeplz-zz

-print out length of title and number of columns in error message

@icanhazcodeplz-zz

-print out length of title and number of columns in error message -added test case for when layout=(2,2) but number of columns=3

@icanhazcodeplz-zz

Conflicts:

doc/source/whatsnew/v0.20.0.txt

TomAugspurger

# Case len(title) == len(df)
plot = df.plot(subplots=True, title=title)
self.assertEqual([p.title._text for p in plot], title)

Choose a reason for hiding this comment

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

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().

Choose a reason for hiding this comment

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

Oh cool I didn't know that existed! Just pushed with the change.

sinhrks

@icanhazcodeplz-zz

TomAugspurger

@TomAugspurger

Great thanks. +1 for merge when travis passes. Just ping us if you notice that it's green before we do

@icanhazcodeplz-zz

jorisvandenbossche

Choose a reason for hiding this comment

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

@bmagnusson few more minor (stylistic) comments

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' \

Choose a reason for hiding this comment

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

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'

Choose a reason for hiding this comment

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

same here

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

Choose a reason for hiding this comment

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

single backticks (`) around 'subplots' (to make it clear that it is a keyword)

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

Choose a reason for hiding this comment

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

Can you leave the starting sentence "Title to use for the plot" ?

@icanhazcodeplz-zz

…ing.

-A few other stylistic changes for consistency.

@icanhazcodeplz-zz

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?

@icanhazcodeplz-zz

@icanhazcodeplz-zz

@icanhazcodeplz-zz

Ah! I figured it out. I had a linting error (I'll start doing lint before committing oops!)

@icanhazcodeplz-zz

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

yarikoptic added a commit to neurodebian/pandas that referenced this pull request

Dec 12, 2016

@yarikoptic

yarikoptic added a commit to neurodebian/pandas that referenced this pull request

Dec 12, 2016

@yarikoptic

release 0.19.1 was from release branch

ischurov pushed a commit to ischurov/pandas that referenced this pull request

Dec 19, 2016

@bmagnusson @ischurov

…lotting (pandas-dev#14753)

Labels