-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
ENH/CLN: add HistPlot class inheriting MPLPlot #7809
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
Conversation
|
does this change anything? or is just an internal refactor? what is more consistent? |
|
this independent of #7717 ? |
|
It looks like the main API change is that as of this PR you can make a histogram with |
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.
needs to add this to the doc strings for plot_series/frame, no?
|
@sinhrks also let's add a release note in API section detailing this change/enhancement |
|
Will do. Added example and remaining items on top. |
|
is |
|
change the title on this as well (the PR), no longer a WIP, rigth? |
|
Going to leave |
|
I think this can be reviewed (though |
|
so #7844 should be merged first? or does this include as well? |
|
Yes, #7844 should be first. |
|
this looked ok, @sinhrks can you rebase @TomAugspurger have a look? |
|
OK, rebased. |
|
I just gave it a quick look (didn't test, computer was stolen!) and I think it looks good. Only comment was maybe setting a random seed in the doc example, very minor. Nice work!
|
|
@sinhrks hmm, needs rebasing again, then ok to merge |
|
ping when pushed |
|
@jreback Rebased and got green. |
ENH/CLN: add HistPlot class inheriting MPLPlot
|
thanks! |
Because
histandboxplotare separated from normalplot, there are some inconsistencies with these functions. Looks better to include them toMPLPlotframework.Maybe
scatterandhistcan be deprecated in 0.15 ifMPLPlotcan offer betterGroupByplot (plan to do in separate PR).Example
This allows to use
kind='histinDataFrame.plotandSeries.plot. (No changes forDataFrame.hist)Remaining Items
rotandfontsize(depending onorientationkw) (rely on BUG/VIS: rot and fontsize are not applied to timeseries plots #7844)stacked=Truecan be supported (DataFrame.histdoesn't support it though..).