Skip to content
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: Add area plot for DataFrame #696

Merged
merged 5 commits into from
Aug 28, 2019

Conversation

charlesdong1991
Copy link
Contributor

@charlesdong1991 charlesdong1991 commented Aug 26, 2019

kdf plot:

Screen Shot 2019-08-26 at 2 57 11 PM

pdf plot:

Screen Shot 2019-08-26 at 2 57 18 PM

@charlesdong1991 charlesdong1991 changed the title Add area plot for DataFrame ENH: Add area plot for DataFrame Aug 26, 2019
Returns
-------
matplotlib.axes.Axes or numpy.ndarray
Area plot, or array of area plots if subplots is True.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charlesdong1991, after you finish to add all plots, mind cleaning those documentation to have some examples in a separate PR later? It would be great if we do it in Seres's too together.

Copy link
Contributor Author

@charlesdong1991 charlesdong1991 Aug 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, honestly I was thinking of doing so :) once most of plots are finished, I will create a separate PR to fix them all for Series and DataFrame. I will focus on plot implementation for now.

Does it sound good to you? @HyukjinKwon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, plase!

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. Let me double check tomorrow and merge.

@codecov-io
Copy link

codecov-io commented Aug 26, 2019

Codecov Report

Merging #696 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #696   +/-   ##
=======================================
  Coverage   93.87%   93.87%           
=======================================
  Files          32       32           
  Lines        5548     5548           
=======================================
  Hits         5208     5208           
  Misses        340      340
Impacted Files Coverage Δ
databricks/koalas/plot.py 94.79% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c7028e...496e639. Read the comment docs.

@HyukjinKwon
Copy link
Member

@charlesdong1991, mind resolving conflicts?

@charlesdong1991
Copy link
Contributor Author

yeah, resolved. @HyukjinKwon

@charlesdong1991
Copy link
Contributor Author

i will resolve the conflict first, and test the examples used on Pandas to make sure the implementation is correct... I will ping when this PR is ready... @HyukjinKwon

@HyukjinKwon
Copy link
Member

Oops, yes. please resolve conflicts. I will merge.

@softagram-bot
Copy link

Softagram Impact Report for pull/696 (head commit: 496e639)

⭐ Change Overview

Showing the changed files, dependency changes and the impact - click for full size
(Open in Softagram Desktop for full details)

📄 Full report

Give feedback on this report to [email protected]

@charlesdong1991
Copy link
Contributor Author

looks good with pandas plot example:

Screen Shot 2019-08-28 at 9 29 19 AM

works good with y specified:

Screen Shot 2019-08-28 at 9 29 34 AM

works also good with stacked=False

Screen Shot 2019-08-28 at 9 29 28 AM

i also added tests for the latter two cases in tests to prove they all work correctly. @HyukjinKwon

@HyukjinKwon
Copy link
Member

nice!

@HyukjinKwon HyukjinKwon merged commit 19a32f0 into databricks:master Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants