fixing pandas.DataFrame.plot(): labels do not appear in legend and label kwd
State: closed

Closes https://github.com/pydata/pandas/issues/9542, #8905

The following behavior has been tested: 
- `df.plot(y='sin(x)')` -> gives a legend with label 'None' -> this should give no legend instead (as it plots one series, and then we don't automatically add a legend, see behaviour of df['sin(x)'].plot())
- `df.plot(y='sin(x)', legend=True)` -> gives a legend with label 'None' -> this should of course give a legend with label 'sin(x)' (behaviour as df['sin(x)'].plot(legend=True))
- `df.plot(y='sin(x)', label='something else', legend=True)` -> gives a legend with label 'None' -> should be a legend with label 'something else', as we want that the label kwarg overwrites the column name.

based on following data:
`x=np.linspace(-10,10,201)`
`y,z=np.sin(x),np.cos(x)`
`x,y,z=pd.Series(x),pd.Series(y),pd.Series(z)`
`df=pd.concat([x,y,z],axis=1)`
`df.columns=['x','sin(x)','cos(x)']`
`df=df.set_index('x')`


Labels:
name: Bug
name: Visualization
description: plotting

Comments:
not sure why there would be a test_url failure in the build... also not used to Travis - did the build properly incorporate both commits?

@schmohlio I think you can ignore that test_url error, it does not seem to be related to this PR.

You can indeed put the tests in `test_graphics`. 
I did not yet look in detail, but a quick feedback: I don't think it should be necessary to change the default value for `legend`. Further, is it needed to change the existing tests?

cc @TomAugspurger @sinhrks 

Thanks for looking @jorisvandenbossche 

Is there an elegant way to check for user supplied arg (legend=) when default is not None?

Didn't spend too much time on tests, but I made changes to align plot_frame with plot_series. Previously, label wasn't getting passed through at all + it was overwriting index name within plot_frame. Per #9542, it probably doesn't make sense to mutate the index name with `label=`, which is reflected in test changes. That previous design flaw was exacerbated when composite .plot calls are made, and the x axis only uses `label=` arg from the latest call.

Will await more feedback @TomAugspurger @sinhrks . Thanks again for letting me contribute!

I'll try to look more later.

Good catch. 

I agree with Joris that the default for legend doesn't need to change.

@jorisvandenbossche @TomAugspurger - here is a summary. Pushing the following in a few:
1. Revert to default `legend=True`. 
   - this means that even if no `label=` arg is provided by the user and only 1 series is plotted, the legend will still show something based on #2. This will just be a difference for Series and DataFrames. 
   - Previously I had introduced logic based on the number of series.
   - will revert back `legend=True` on test edits
2. Ensure the following logic for coalescing the legend labels:
   - check `label=` arg (and make sure it is passed through)--> check series name property (y label, column name, etc)  
3. Keep index name test removed, and keep other tests that check legend. Maybe expand upon these.

Thanks for the summary. That all looks perfect.

When you push again could you squash things down to a single commit and add a note in `doc/source/whatsnew/v0.16.0`. Thanks.

I added tests to the proper check legend labels method. Altogether, this test method validates all the changes we discussed. Now the `label=` arg functions intuitively. I also added a check to make sure there is no column mutation. Thanks for patience.

Pushing to branch now @TomAugspurger @jorisvandenbossche 

Great thanks. If you could just rebase + squash and add a release not I'll get it merged.

Hey @TomAugspurger Tom - just squashed my commits, rebased, pulled, and pushed. Hope that's okay.

Is it possible to keep the behaviour as before? So that when plotting one series by specifying the `y` kwarg no legend is shown? (although I don't know if this is the best behaviour, but when plotting one series, it looks to me as the name of the series should be the `ylabel`)

@jorisvandenbossche - right now a legend will appear with the ylabel as the label, unless the user explicitly supplies legend=False. I agreed before but seeing it in action now it almost seems intuitive to have the legend default to true. If we are going to keep the default to True it seems difficult to cleanly override defaults in case the user wants the legend. 

The tests also line up, seemingly making this the smallest incremental change that fixes the labels not getting passed through. What if we added another ticket for adding a legend to series plots? 

@TomAugspurger I think I see what you mean by release notes- will add those. does everything else look ok?

@TomAugspurger I added release notes and squashed my commits properly. Figured we could go from there in a cleaner manner.

I think we're really close. Code-wise everything looks good. Just those couple things in the release notes. If you want to just rebase _one_ more time :)

I'll be in our chat room at https://gitter.im/pydata/pandas Give me a shout if you have any trouble.

And @schmohlio looks like you'll be one of the first ones in `0.16.1`. Could you make a new file at `doc/source/whatsnew/v0.16.1.txt` And paste this in:

EDIT: the formatting was a bit odd here. Paste in the text from this gist: https://gist.github.com/TomAugspurger/7f5061219243a24ac31f

Then you can move your section in `doc/source/whatsnew/v0.16.0` to that new file. Thanks.

cool this looks good. will shorten and rebase with new template.

whats new is now available for 0.16.1 https://github.com/pydata/pandas/commit/5ebf521eac528f5a4b772e4677f70b689b8b1826

@jreback should i remove the whatsnew file I had added?

yes, when you pull and rebase you will get the new one

Thanks @schmohlio!

Looks like this was your first PR for pandas. Stick around if you find some more bugs :)

@TomAugspurger thanks to you! will look out for sure.