feat: (Series|Dataframe).plot.hist()#420
Conversation
06fb1e6 to
84f816d
Compare
84f816d to
a2ac563
Compare
| import pandas as pd | ||
|
|
||
| import bigframes.constants as constants | ||
| from bigframes.operations._matplotlib.core import MPLPlot |
There was a problem hiding this comment.
import modules not methods/classes.
https://google.github.io/styleguide/pyguide.html#s2.2-imports
bigframes/operations/plot.py
Outdated
| ) | ||
| kwargs["by"] = by | ||
| kwargs["bins"] = bins | ||
| return plotbackend.plot(self._parent.copy(), kind="hist", **kwargs) |
There was a problem hiding this comment.
Could you explain a bit more in the comments what's going on here?
Do we need to add a TODO for supporting other plotbackends, for example?
There was a problem hiding this comment.
Added a comment here. We may introduce other backends for interactive visualization support in the future.
| "tabulate >= 0.9", | ||
| "ipywidgets >=7.7.1", | ||
| "humanize >= 4.6.0", | ||
| "matplotlib >= 3.7.1", |
There was a problem hiding this comment.
Please add this to https://github.com/googleapis/python-bigquery-dataframes/blob/main/testing/constraints-3.10.txt too so that we know we always test against our advertised minimum version in at least one test session.
There was a problem hiding this comment.
matplotlib==3.7.1 has been added into this file earlier. The system-3.10 tests are not triggerred according to
python-bigquery-dataframes/noxfile.py
Line 57 in 60594f4
If so, should I add the matplotlib to contraints-3.9.txt also?
There was a problem hiding this comment.
Oh, you're right. Yes add to contraints-3.9.txt. I was looking at the list of files sorted alphabetically.
| return strings.StringMethods(self._block) | ||
|
|
||
| @property | ||
| def plot(self): |
There was a problem hiding this comment.
Docstring, please.
Also, please add PlotAccessor to https://github.com/googleapis/python-bigquery-dataframes/blob/main/docs/reference/bigframes.pandas/series.rst
There was a problem hiding this comment.
Thanks! Added PlotAccessor to the series.rst and frames.rst.
As for the docstring for def plot, it is defined at third_party/bigframes_vendored/pandas/core/series.py
a2ac563 to
82797de
Compare
39b6c63 to
e71d19c
Compare
| .. automodule:: bigframes.operations.plotting | ||
| :members: | ||
| :inherited-members: | ||
| :undoc-members: |
There was a problem hiding this comment.
Per https://github.com/googleapis/python-bigquery-dataframes/actions/runs/8240146608/job/22534948373?pr=420 failure, let's add :noindex: here.
There was a problem hiding this comment.
Thanks for checking! Fixed.
|
|
||
| self.axes = hist_x_pd.plot.hist( | ||
| bins=bin_edges, | ||
| weights=np.array(weights_pd.values), |
There was a problem hiding this comment.
Is this method still not working on pandas 1.5 or did the fillna solve that?
There was a problem hiding this comment.
the fillna solves the issue in pandas 1.5. I can add a comment above calling.
internal bug: b/322178004