Skip to content

Conversation

@ueshin
Copy link
Collaborator

@ueshin ueshin commented Jan 27, 2020

We should not have renamed the columns in _InternalFrame.with_new_columns, otherwise the new column names could be unexpectable.

@ueshin ueshin requested a review from HyukjinKwon January 27, 2020 21:43
@codecov-io
Copy link

codecov-io commented Jan 27, 2020

Codecov Report

Merging #1229 into master will increase coverage by 1.42%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1229      +/-   ##
==========================================
+ Coverage   93.85%   95.27%   +1.42%     
==========================================
  Files          35       35              
  Lines        7271     7393     +122     
==========================================
+ Hits         6824     7044     +220     
+ Misses        447      349      -98
Impacted Files Coverage Δ
databricks/koalas/internal.py 95.57% <100%> (ø) ⬆️
databricks/koalas/groupby.py 91.76% <100%> (ø) ⬆️
databricks/koalas/indexing.py 97.26% <0%> (+0.94%) ⬆️
databricks/conftest.py 98% <0%> (+4%) ⬆️
databricks/koalas/__init__.py 85.1% <0%> (+6.38%) ⬆️
databricks/koalas/usage_logging/usage_logger.py 100% <0%> (+50%) ⬆️
databricks/koalas/usage_logging/__init__.py 97.29% <0%> (+72.97%) ⬆️

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 1c973a5...051aba3. Read the comment docs.

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.

LGTM

self._groupkeys_scols = [F.col(name_like_string(s.name)) for s in self._groupkeys]
self._agg_columns_scols = [F.col(name_like_string(s.name)) for s in self._agg_columns]
self._groupkeys_scols = [F.col(s._internal.data_columns[0]) for s in self._groupkeys]
self._agg_columns_scols = [F.col(s._internal.data_columns[0]) for s in self._agg_columns]
Copy link
Member

Choose a reason for hiding this comment

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

I think now we can enable the tests back (#1224 (comment)). I will make a small followup.

@HyukjinKwon HyukjinKwon merged commit 5d8fbb1 into databricks:master Jan 28, 2020
@ueshin ueshin deleted the fix_with_new_columns branch January 28, 2020 01:23
ueshin pushed a commit that referenced this pull request Jan 28, 2020
…different values (#1233)

A small followup of #1224 and #1229

Now, we can cover this case
rising-star92 added a commit to rising-star92/databricks-koalas that referenced this pull request Jan 27, 2023
…different values (#1233)

A small followup of databricks/koalas#1224 and databricks/koalas#1229

Now, we can cover this case
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.

3 participants