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

Fix renaming on index to_frame method #4498

Merged
merged 9 commits into from Mar 27, 2019

Conversation

Projects
None yet
4 participants
@henriqueribeiro
Copy link
Contributor

commented Feb 18, 2019

  • Tests added / passed
  • Passes flake8 dask

Fix issue #3564

Finally pandas 0.24 was released and so, the fix for index to_frame bug is now patched. However it was not possible to replicate pandas behavior since map_partitions don't have access to global index when index=False

Show resolved Hide resolved dask/dataframe/core.py Outdated
@henriqueribeiro

This comment has been minimized.

Copy link
Contributor Author

commented Feb 18, 2019

@TomAugspurger I believe this is it

@henriqueribeiro

This comment has been minimized.

Copy link
Contributor Author

commented Feb 19, 2019

Do you agree with the error message?

@jrbourbeau
Copy link
Member

left a comment

Thanks for this contribution @henriqueribeiro! Looking forward to seeing it added

Show resolved Hide resolved dask/dataframe/core.py Outdated
Show resolved Hide resolved dask/dataframe/core.py Outdated
Show resolved Hide resolved dask/dataframe/tests/test_indexing.py Outdated
@mrocklin

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

@henriqueribeiro

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2019

So you agree that we should leave the index always set to True?

@TomAugspurger

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

@henriqueribeiro do you have time to update based on the discussion in
#4498 (comment)?

@henriqueribeiro

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

yes sure, I will do it.

@henriqueribeiro

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 2019

Everything seems to be fine now

@henriqueribeiro

This comment has been minimized.

Copy link
Contributor Author

commented Mar 25, 2019

@jrbourbeau do you agree with this change?

@jrbourbeau
Copy link
Member

left a comment

Yes, this LGTM. Thanks @henriqueribeiro!

@jrbourbeau jrbourbeau merged commit 77af7c8 into dask:master Mar 27, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

asmith26 added a commit to asmith26/dask that referenced this pull request Apr 22, 2019

Fix renaming on index to_frame method (dask#4498)
* add new method for to_frame function on type dask.dataframe.core.Index

* cannot handle index argument from to_frame pandas function

* add tests for index to_frame

* verify pandas version

* change the way pandas version is verified and handled

* create specific tests for renaming index

* raise error if pandas version lower than 0.21

* do not test for pandas lower than 0.21

* use the same prototype as pandas but raising error if index is False

jorge-pessoa pushed a commit to jorge-pessoa/dask that referenced this pull request May 14, 2019

Fix renaming on index to_frame method (dask#4498)
* add new method for to_frame function on type dask.dataframe.core.Index

* cannot handle index argument from to_frame pandas function

* add tests for index to_frame

* verify pandas version

* change the way pandas version is verified and handled

* create specific tests for renaming index

* raise error if pandas version lower than 0.21

* do not test for pandas lower than 0.21

* use the same prototype as pandas but raising error if index is False
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.