ENH: Allow column grouping in DataFrame.plot
State: closed

- [X] closes #29688
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

This is an attempt at #29688 i.e. group some columns on the same ax when calling `df.plot`

See [this gist](https://gist.github.com/NicolasHug/4d1f60212ba69e6000982951ae47f203) for illustration of different supported plots

Thanks!

Labels:
name: Enhancement
name: Visualization
description: plotting

Comments:
@NicolasHug is this still active? Looks like a merge conflict and CI failure need to be addressed
I'm still happy to work on this. I just haven't received initial feedback yet (and don't want to push either, I understand this isn't necessarily a high priority)

> Before working more on this I'd appreciate some feedback to make sure this is going the in right direction, and that this is actually a desired feature.
Feel free to close otherwise, or LMK if you need to see more to decide :)
Understood. I doubt anyone has reviewed since CI has been red, so definitely want to get that green to start
Will do
@charlesdong1991 any thoughts on this?
Thanks a lot for the review @charlesdong1991 , I addressed your comments and added a bunch of validation / tests.

It looks like the code works for most kinds of plots. You can see a few illustrations in [this gist](https://gist.github.com/NicolasHug/4d1f60212ba69e6000982951ae47f203).

Looks like the subplot titles could be improved in some cases, I'll see what I can do. But otherwise I think this is ready for another round ;)
Regarding the docstring, I'm a bit confused because it seems that `subplots` have been removed from most docstrings? i.e. `git grep "subplots :"` only gives one hit, for `boxplot_frame_groupby()`. I can't find it for `PlotAccessor`.
@NicolasHug can you merge master and update to comments
sorry I'm short on time ATM and I won't be able to come back to this before a few weeks
Thanks for this nice proposal, it definitely will be nice to have!!

and I am very sorry for taking so long for the feedbacks from my side.

not sure if I am allowed to finish this PR for you to make sure you have the credit as well.  I will take a look and see!
no worries at all, I know what it's like ;)

I definitely plan to come back to it later, but if you'd rather have this in sooner then feel free to take over!
ahh, great!! then will leave it on you, and wish to see you coming back to implement this nice feature! ^^  @NicolasHug 
thanks for the review and for the tips @simonjayhawkins , I have updated the PR accordingly.
The test failure seems to be related to a networking issue (unclosed socket), I'm not sure this is directly related to this PR?
Looks like the CI failure was magically resolved. I think this is ready for another round! thanks for the reviews so far.
> can you show a sample plot with this api.

@NicolasHug indeed made a nice sample ^^, see in PR description on top: 
> See this gist for illustration of different supported plots

the link is https://gist.github.com/NicolasHug/4d1f60212ba69e6000982951ae47f203  
there was a discussion about pie plots since they don't as good as others, https://github.com/pandas-dev/pandas/pull/29944#discussion_r404364673 @jreback your opinion is very welcomed!
Thanks @jreback and @charlesdong1991 . All of your requested changes were addressed.
> are there other ways to do this, e. g a for loop?

I know of no other way in pandas. Like any other supported plot in pandas, this is of course possible with pure matplotlib code, but with more boilerplate.
I'm going to stop addressing merge conflicts because they seem to occur 5 minutes after every single commit that I make. They're only about conflicts in the imports. I hope this doesn't hurt reviewing.
thanks @NicolasHug , IMHO, this is a very nice PR and is very close to be merged, many thanks for your patience and awesome work!

i think the conflict won't affect your PR, so it's okay to resolve the conflict once @jreback comments on https://github.com/pandas-dev/pandas/pull/29944#issuecomment-687559969 and there is kinda consensus on how to address.

One other thing worth mentioning is https://github.com/pandas-dev/pandas/pull/29944#discussion_r483902388 the whatsnew of this PR should be moved to  `Plotting` section in `1.2` instead of `1.1.2`, since `1.1.2` will be mainly focused on regression related issues.  Therefore, I un-resolved the conversation which was resolved by you.
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
i'm still happy to address any future review
@NicolasHug thanks for being patient! Can you merge master one more time and resolve conflicts?

cc @jreback  xref https://github.com/pandas-dev/pandas/pull/29944#issuecomment-687559969
cc @jreback @charlesdong1991 this looks close to going in in case we want it for 1.2
ping @jreback @charlesdong1991 
Hello @NicolasHug! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:







There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2022-06-03 19:13:34 UTC
Thank you for the review @jreback, comments were addressed.

> don't we already do this when we df.groupb(...).plot.line() for example?

I'm probably missing something but `groupby.plot` does something very different from the proposed PR: `groupby.plot` will group by **value** while the proposed PR simply groups **entire columns** within the same subplots.
CI Failures are unrelated I think
> CI Failures are unrelated I think

```
Performing static analysis using mypy
pandas/plotting/_matplotlib/core.py:304: error: Incompatible return value type (got "List[Tuple[int, ...]]", expected "Union[bool, List[Tuple[int]]]")  [return-value]
Found 1 error in 1 file (checked 1177 source files)
Performing static analysis using mypy DONE
```

mypy needs to be green before we can consider merging. 
Thanks, fixed mypy. Should `Linux py38_np_dev` still fail, I believe it's still unrelated to the proposed changes.
@NicolasHug Mind merging master/resolving conflicts? This would be quite nice to get into 1.3 I think.