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.

Sign up for GitHub

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

Jump to bottom

Fix to delegate dtypes. #2061

Merged
merged 5 commits into from
Mar 3, 2021
Merged

Fix to delegate dtypes. #2061

merged 5 commits into from
Mar 3, 2021

Conversation

ueshin
Copy link
Collaborator

@ueshin ueshin commented Feb 20, 2021

Fixes to delegate dtypes properly.
The parameter preserve_dtypes for the workaround was removed.
Also maked as TODO where the dtypes are unknown.

@ueshin ueshin requested review from itholic, xinrong-meng and HyukjinKwon and removed request for itholic and xinrong-meng February 22, 2021 19:24
@ueshin ueshin marked this pull request as ready for review February 22, 2021 19:24
@ueshin ueshin mentioned this pull request Feb 25, 2021
Merged
@codecov-io
Copy link

codecov-io commented Feb 27, 2021 •
edited
Loading

Codecov Report

Merging #2061 (7137266) into master (0f6eb80) will decrease coverage by 0.79%.
The diff coverage is 99.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2061      +/-   ##
==========================================
- Coverage   94.68%   93.89%   -0.80%     
==========================================
  Files          55       55              
  Lines       11718    11755      +37     
==========================================
- Hits        11095    11037      -58     
- Misses        623      718      +95
Impacted Files Coverage Δ
databricks/koalas/mlflow.py 95.23% <ø> (ø)
databricks/koalas/utils.py 95.45% <ø> (ø)
databricks/koalas/series.py 96.78% <92.30%> (ø)
databricks/koalas/accessors.py 93.46% <100.00%> (ø)
databricks/koalas/base.py 97.53% <100.00%> (ø)
databricks/koalas/frame.py 96.61% <100.00%> (+0.02%) ⬆️
databricks/koalas/groupby.py 91.63% <100.00%> (+0.02%) ⬆️
databricks/koalas/indexes/base.py 97.42% <100.00%> (+<0.01%) ⬆️
databricks/koalas/indexes/multi.py 93.83% <100.00%> (+0.02%) ⬆️
databricks/koalas/indexing.py 92.83% <100.00%> (ø)
... and 9 more

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 0f6eb80...7137266. Read the comment docs.

xinrong-meng
xinrong-meng reviewed Mar 3, 2021
View reviewed changes
databricks/koalas/series.py
@@ -2715,7 +2713,9 @@ def add_prefix(self, prefix) -> "Series":
]
+ internal.data_spark_columns
)
return first_series(DataFrame(internal.with_new_sdf(sdf)))
return first_series(
DataFrame(internal.with_new_sdf(sdf, index_dtypes=([None] * internal.index_level)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we pass objects as index_dtypes directly?

Copy link
Collaborator Author

@ueshin ueshin Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

If we pass None, the dtype will be inferred during the initialization of InternalFrame.

@xinrong-meng
Copy link
Contributor

I'm wondering if we wanted to call out spark_type_to_pandas_dtype is where the type mapping defined in internal.py.
I'm afraid contributors with little context might not be clear about this.

@xinrong-meng
Copy link
Contributor

Looks great! Left some questions. Thank you!

@ueshin
Copy link
Collaborator Author

ueshin commented Mar 3, 2021

Thanks! let me merge this now. Please feel free to leave comments later if any.

@ueshin ueshin merged commit 9bac655 into databricks:master Mar 3, 2021
@ueshin ueshin deleted the type_mapping branch March 3, 2021 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xinrong-meng xinrong-meng xinrong-meng approved these changes

@itholic itholic Awaiting requested review from itholic

@HyukjinKwon HyukjinKwon Awaiting requested review from HyukjinKwon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ueshin @codecov-io @xinrong-meng