-
Notifications
You must be signed in to change notification settings - Fork 358
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: Implement frame plot #686
ENH: Implement frame plot #686
Conversation
Codecov Report
@@ Coverage Diff @@
## master #686 +/- ##
==========================================
+ Coverage 93.52% 93.68% +0.15%
==========================================
Files 32 32
Lines 5455 5492 +37
==========================================
+ Hits 5102 5145 +43
+ Misses 353 347 -6
Continue to review full report at Codecov.
|
table=table, yerr=yerr, xerr=xerr, secondary_y=secondary_y, | ||
sort_columns=sort_columns, **kwds) | ||
|
||
def line(self, x=None, y=None, **kwargs): |
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.
Can we mimic pandas documenation (https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.plot.line.html#pandas.DataFrame.plot.line)? It's okay to don't include the images for now.
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.
Thanks @charlesdong1991.
-
Can you add a test to test missing plot implementations? See
test_series_plot.py
def test_missing(self): ks = self.kdf1['a'] unsupported_functions = ['kde'] for name in unsupported_functions: with self.assertRaisesRegex(PandasNotImplementedError, "method.*Series.*{}.*not implemented".format(name)): getattr(ks.plot, name)()
-
Can you show an example code with output? (see Add plot.area in Series #670)
-
We should create and update plot in documentation (see https://github.com/databricks/koalas/pull/670/files#diff-9cc87824986b3722f2133fdb0023fb69R332). Let's match it to https://pandas.pydata.org/pandas-docs/stable/reference/frame.html#plotting
Hi, @HyukjinKwon thanks for taking a look at the PR, and I changed the code to reflect your points in the review. And here I attach the output with the test example code. |
Softagram Impact Report for pull/686 (head commit: 14a55b5)⭐ Change Overview
📄 Full report
Give feedback on this report to [email protected] |
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.
Looks fine except, do you mind if I ask to post pandas example (input/output) too in PR description as well? Just want to make sure that it shows the same chart.
thanks for your feedback, fair enough! I added it in the description. Once it's merged, then I will add other sub modules for frame plot. @HyukjinKwon |
Merged to master. Thanks you for addressing my comments. |
This PR is mainly creating frame plot which is kind of base method, now, only line plot is added.
koalas plot:
pandas plot: