-
-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
ENH/CLN: add HistPlot class inheriting MPLPlot #7809
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 |
# kinds supported by both dataframe and series | ||
_common_kinds = ['line', 'bar', 'barh', 'kde', 'density', 'area'] | ||
_common_kinds = ['line', 'bar', 'barh', 'kde', 'density', 'area', 'hist'] |
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
hist
andboxplot
are separated from normalplot
, there are some inconsistencies with these functions. Looks better to include them toMPLPlot
framework.Maybe
scatter
andhist
can be deprecated in 0.15 ifMPLPlot
can offer betterGroupBy
plot (plan to do in separate PR).Example
This allows to use
kind='hist
inDataFrame.plot
andSeries.plot
. (No changes forDataFrame.hist
)Remaining Items
rot
andfontsize
(depending onorientation
kw) (rely on BUG/VIS: rot and fontsize are not applied to timeseries plots #7844)stacked=True
can be supported (DataFrame.hist
doesn't support it though..).