Skip to content

Conversation

@shril
Copy link
Contributor

@shril shril commented May 12, 2019

@rxin, @ueshin Please have a look at my PR. #169
Happy to help regarding my changes.

Reference Pandas Doc: https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.DataFrame.to_records.html

@codecov-io
Copy link

codecov-io commented May 12, 2019

Codecov Report

Merging #298 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #298      +/-   ##
==========================================
+ Coverage   94.31%   94.33%   +0.02%     
==========================================
  Files          35       35              
  Lines        3551     3564      +13     
==========================================
+ Hits         3349     3362      +13     
  Misses        202      202
Impacted Files Coverage Δ
databricks/koalas/missing/frame.py 100% <ø> (ø) ⬆️
databricks/koalas/testing/utils.py 74.05% <100%> (+0.33%) ⬆️
...tabricks/koalas/tests/test_dataframe_conversion.py 100% <100%> (ø) ⬆️
databricks/koalas/frame.py 95.46% <100%> (+0.03%) ⬆️

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 926400f...edf4408. Read the comment docs.

@shril
Copy link
Contributor Author

shril commented May 12, 2019

@rxin I am getting this exception for Pandas 0.23.4

UNEXPECTED EXCEPTION: TypeError("The pandas version [0.23.4] available does not support parameter 'column_dtypes' for function 'to_records'.",)

Should I skip doctest for it.

@shril
Copy link
Contributor Author

shril commented May 13, 2019

@rxin, @ueshin I think I need some help here.

@garawalid
Copy link
Contributor

@shril I suggest that you delete the parameter column_dtypes from args if the pandas version is less than 0.23.4

if LooseVersion(pd.__version__) < LooseVersion("0.24.0"):
    del args['column_dtypes'] 

@shril
Copy link
Contributor Author

shril commented May 14, 2019

@shril I suggest that you delete the parameter column_dtypes from args if the pandas version is less than 0.23.4

if LooseVersion(pd.__version__) < LooseVersion("0.24.0"):
    del args['column_dtypes'] 

Thanks again @garawalid

@rxin
Copy link
Contributor

rxin commented May 14, 2019

@shril this doesn't merge cleanly anymore. Can you merge master into your branch so this can merge cleanly?

@HyukjinKwon
Copy link
Member

Yea, many PRs are rapidly being merged. I will take a look as soon as it's merged to master.

@shril
Copy link
Contributor Author

shril commented May 19, 2019

@rxin @HyukjinKwon I will look into this today.
Sorry for the delay.

@shril
Copy link
Contributor Author

shril commented May 19, 2019

@rxin, @HyukjinKwon I have made the required changes.
I apologize for the delay. Please have a look.
Happy to address any issues regarding my changes.

PR #169.

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 otherwise.

>>> df.to_records(index_dtypes="<S2") # doctest: +SKIP
rec.array([(b'a', 1, 0.5 ), (b'b', 2, 0.75)],
dtype=[('index', 'S2'), ('A', '<i8'), ('B', '<f8')])
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@shril shril May 19, 2019

Choose a reason for hiding this comment

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

@HyukjinKwon, I am having this issue that dtypes were added in Pandas 0.24.0. So the doc-tests are failing for the lower version of Pandas. So I am skipping the tests for them although I have pushed unit tests for every variation of the function.

@HyukjinKwon HyukjinKwon merged commit 2cc79bf into databricks:master May 19, 2019
@HyukjinKwon
Copy link
Member

Thanks, @shril

@shril
Copy link
Contributor Author

shril commented May 19, 2019

You are most welcome @HyukjinKwon.

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