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

Clean pandas usage in frame.agg #821

Merged
merged 2 commits into from
Sep 23, 2019

Conversation

charlesdong1991
Copy link
Contributor

@charlesdong1991 charlesdong1991 commented Sep 23, 2019

I came across the frame.agg when i read through code base, and based on the inline comment, I think the manipulation using pandas could be simplified a bit IIUC.

Screen Shot 2019-09-23 at 10 34 55 PM

@codecov-io
Copy link

codecov-io commented Sep 23, 2019

Codecov Report

Merging #821 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #821      +/-   ##
==========================================
- Coverage   94.36%   94.36%   -0.01%     
==========================================
  Files          32       32              
  Lines        5854     5853       -1     
==========================================
- Hits         5524     5523       -1     
  Misses        330      330
Impacted Files Coverage Δ
databricks/koalas/frame.py 96.89% <100%> (-0.01%) ⬇️

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 3e263df...a23fc27. Read the comment docs.

Copy link
Collaborator

@ueshin ueshin left a comment

Choose a reason for hiding this comment

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

This is great. The code is really simpler.

lambda gpdf: gpdf.drop('level_1', 1).set_index('level_0').transpose()
).reset_index(level=1)
pdf = pdf.drop(columns='level_1')
pdf = kdf.to_pandas().stack(level=1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can just use .stack() here? Then I guess we can reuse when supporting multi-index columns.

Copy link
Contributor Author

@charlesdong1991 charlesdong1991 Sep 23, 2019

Choose a reason for hiding this comment

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

yeah, .stack() should also work indeed. i was trying to be more explicit. Use .stack() to facilitate supporting multiindex, will change!

thanks! @ueshin

@softagram-bot
Copy link

Softagram Impact Report for pull/821 (head commit: a23fc27)

⭐ Change Overview

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

📄 Full report

Impact Report explained. Give feedback on this report to [email protected]

Copy link
Collaborator

@ueshin ueshin left a comment

Choose a reason for hiding this comment

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

LGTM, pending tests.

@ueshin
Copy link
Collaborator

ueshin commented Sep 23, 2019

Thanks! merging.

@ueshin ueshin merged commit 5e39ad5 into databricks:master Sep 23, 2019
@HyukjinKwon
Copy link
Member

Nice! Thanks!

thoo added a commit to thoo/koalas that referenced this pull request Sep 24, 2019
* upstream/master:
  Updated the koalas logo in readme.md
  Adding koalas-logo without label
  Adding Koalas logo to readme
  Adding koalas logo
  Clean pandas usage in frame.agg (databricks#821)
  Implement Series.aggregate and agg (databricks#816)
  Raise a more helpful error for duplicated columns in Join (databricks#820)
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.

5 participants