-
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
Issue with ks.merge to Series #818
Conversation
Codecov Report
@@ Coverage Diff @@
## master #818 +/- ##
=========================================
+ Coverage 94.25% 94.36% +0.1%
=========================================
Files 32 32
Lines 5853 5855 +2
=========================================
+ Hits 5517 5525 +8
+ Misses 336 330 -6
Continue to review full report at Codecov.
|
The reason Travis CI is failing is |
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.
@thoo thanks for working on this.
In that case, we can skip tests with LooseVersion
.
Otherwise, LGTM.
|
||
# suffix | ||
check(lambda left, right: left.merge(right, suffixes=['_left', '_right'], how='outer', | ||
left_index=True, right_index=True), right_ks, right_ps) |
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.
Let's skip tests when pandas is older than 0.24. See the example with LooseVersion
in test_groupby.py
.
* 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)
check(lambda left, right: left.merge(right, how=how), right_ks, right_ps) | ||
check(lambda left, right: left.merge(right, left_on='x', right_on='x', how=how), | ||
# pd.DataFrame.merge with Series is implemented since version 0.24.0 | ||
if LooseVersion(pd.__version__) > LooseVersion("0.24.2"): |
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.
nit: shall we use 0.24.0
if pandas supports since then?
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.
Sure.
Softagram Impact Report for pull/818 (head commit: a1f0937)⭐ Change Overview
📄 Full report
Impact Report explained. 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.
LGTM.
Thanks! merging. |
Fix #817
Also add pytests for merging
ks.DataFrame
toks.Series
.