[REVIEW] Match `DataFrame.set_index` with pandas #6231

isVoid · 2020-09-14T23:21:17Z

Implements:

inplace, append, verify_integrity fields to the method
New test cases for these fields.

Addresses Issue 6198 - Adds support for `inplace` field - Optimized logics TODO: - Add test cases - Further support `append` and `verify_integirty` field

…6198

GPUtester · 2020-09-14T23:21:49Z

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

…6198

TODO: - verify_integrity - write tests - input dataframe should include `Index` and `MultiIndex` - separate tests for inplace and verify_integrity PR 6231

TODO: - write tests - input dataframe should include `Index` and `MultiIndex` - separate tests for inplace and verify_integrity - print duplicated keys PR 6231

TODO: - print duplicated keys - support for heterogenous index lists PR 6231

…6198

codecov · 2020-09-17T03:39:33Z

Codecov Report

Merging #6231 into branch-0.16 will increase coverage by 0.27%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           branch-0.16    #6231      +/-   ##
===============================================
+ Coverage        84.45%   84.73%   +0.27%     
===============================================
  Files               82       82              
  Lines            13846    14219     +373     
===============================================
+ Hits             11694    12048     +354     
- Misses            2152     2171      +19

Impacted Files	Coverage Δ
python/dask_cudf/dask_cudf/io/parquet.py	`91.35% <0.00%> (-0.85%)`	⬇️
python/cudf/cudf/_version.py	`44.80% <0.00%> (-0.72%)`	⬇️
python/cudf/cudf/io/parquet.py	`91.73% <0.00%> (-0.50%)`	⬇️
python/cudf/cudf/core/series.py	`90.96% <0.00%> (-0.37%)`	⬇️
python/cudf/cudf/utils/applyutils.py	`98.74% <0.00%> (-0.01%)`	⬇️
python/cudf/cudf/utils/ioutils.py	`85.84% <0.00%> (ø)`
python/cudf/cudf/utils/docutils.py	`100.00% <0.00%> (ø)`
python/dask_cudf/dask_cudf/io/csv.py	`95.52% <0.00%> (ø)`
python/cudf/cudf/core/column/string.py	`86.05% <0.00%> (ø)`
python/dask_cudf/dask_cudf/io/tests/test_csv.py	`100.00% <0.00%> (ø)`
... and 14 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 cb6fb6f...1262e42. Read the comment docs.

python/cudf/cudf/core/dataframe.py

python/cudf/cudf/tests/test_dataframe.py

python/cudf/cudf/core/dataframe.py

- Remove empty line to prevent empty code block - Remove print statement - Parameterize `inplace` Co-authored-by: GALI PREM SAGAR <[email protected]>

Addressing review comment PR 6231

…6198

isVoid · 2020-09-18T01:11:53Z

Where's the problem with CI? Can't figure out the error message.

galipremsagar · 2020-09-18T01:14:04Z

Probably one off issue. Rerun tests might fix it

galipremsagar · 2020-09-18T01:14:11Z

rerun tests

python/cudf/cudf/core/dataframe.py

kkraus14

Couple of minor changes

python/cudf/cudf/core/dataframe.py

- ._set_index default to None - Use MultiIndex.from_frame instead of `source_data` PR 6231

kkraus14 · 2020-09-22T00:52:01Z

@galipremsagar can you review again when you get a chance?

python/cudf/cudf/core/dataframe.py

python/cudf/cudf/tests/test_dataframe.py

python/cudf/cudf/core/dataframe.py

- Code document updates - Code style improvements - Explicitly raising TypeError when input cannot be converted - Throwing KeyError instead of ValueError when column is not found Co-authored-by: GALI PREM SAGAR <[email protected]>

…6198

python/cudf/cudf/core/dataframe.py

- Minor style and doc update. Co-authored-by: GALI PREM SAGAR <[email protected]>

isVoid · 2020-09-23T00:46:30Z

rerun tests

kkraus14 · 2020-09-23T02:37:51Z

@galipremsagar is this good to go once CI reports green? This is blocking a nice dask-cudf groupby optimization 😄

galipremsagar · 2020-09-23T02:38:38Z

@galipremsagar is this good to go once CI reports green? This is blocking a nice dask-cudf groupby optimization 😄

Yep, good to go

…6198

When using RangeIndex, it should not be instantiated unless needed. PR 6231

galipremsagar · 2020-09-23T13:49:58Z

rerun tests

isVoid added 2 commits September 14, 2020 16:13

Initial

1a24b94

Addresses Issue 6198 - Adds support for `inplace` field - Optimized logics TODO: - Add test cases - Further support `append` and `verify_integirty` field

Merge branch 'branch-0.16' of https://github.com/rapidsai/cudf into i…

f39fe3c

…6198

isVoid requested a review from a team as a code owner September 14, 2020 23:21

isVoid added 9 commits September 14, 2020 16:31

Some code changes not commited; Update CHANGELOG

2d3b70e

bug fix

9ec0115

style fix

265fd0a

Merge branch 'branch-0.16' of https://github.com/rapidsai/cudf into i…

6605e95

…6198

Intermediate commit to shift workspace

6c6f44f

Functional append, inplace

b212bc2

TODO: - verify_integrity - write tests - input dataframe should include `Index` and `MultiIndex` - separate tests for inplace and verify_integrity PR 6231

Support verify_integrity

45ab7d5

TODO: - write tests - input dataframe should include `Index` and `MultiIndex` - separate tests for inplace and verify_integrity - print duplicated keys PR 6231

Updated test_set_index

efbcd25

TODO: - print duplicated keys - support for heterogenous index lists PR 6231

Merge branch 'branch-0.16' of https://github.com/rapidsai/cudf into i…

603c84e

…6198

isVoid changed the title ~~[WIP] Match DataFrame.set_index with pandas~~ [REVIEW] Match DataFrame.set_index with pandas Sep 17, 2020

galipremsagar reviewed Sep 17, 2020

View reviewed changes

python/cudf/cudf/core/dataframe.py Show resolved Hide resolved

galipremsagar reviewed Sep 17, 2020

View reviewed changes

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

galipremsagar requested changes Sep 17, 2020

View reviewed changes

python/cudf/cudf/tests/test_dataframe.py Outdated Show resolved Hide resolved

galipremsagar reviewed Sep 17, 2020

View reviewed changes

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

Apply suggestions from code review

dc2d684

- Remove empty line to prevent empty code block - Remove print statement - Parameterize `inplace` Co-authored-by: GALI PREM SAGAR <[email protected]>

kkraus14 added 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer Python Affects Python cuDF API. labels Sep 17, 2020

isVoid added 3 commits September 17, 2020 12:40

Explicitly raise for _set_index type check

62c764b

Addressing review comment PR 6231

Merge branch 'branch-0.16' of https://github.com/rapidsai/cudf into i…

b84b68f

…6198

style fix

3e6a1f9

kkraus14 requested changes Sep 18, 2020

View reviewed changes

kkraus14 requested changes Sep 22, 2020

View reviewed changes

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

Address review comments

9c06ea1

- ._set_index default to None - Use MultiIndex.from_frame instead of `source_data` PR 6231

kkraus14 approved these changes Sep 22, 2020

View reviewed changes

galipremsagar requested changes Sep 22, 2020

View reviewed changes

galipremsagar added 0 - Waiting on Author Waiting for author to respond to review and removed 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer labels Sep 22, 2020

isVoid and others added 3 commits September 21, 2020 20:54

Apply suggestions from code review

88a11e9

- Code document updates - Code style improvements - Explicitly raising TypeError when input cannot be converted - Throwing KeyError instead of ValueError when column is not found Co-authored-by: GALI PREM SAGAR <[email protected]>

Minor style update

80de7c5

Merge branch 'branch-0.16' of https://github.com/rapidsai/cudf into i…

1958419

…6198

isVoid requested a review from galipremsagar September 22, 2020 16:49

galipremsagar added 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer and removed 0 - Waiting on Author Waiting for author to respond to review labels Sep 22, 2020

galipremsagar reviewed Sep 22, 2020

View reviewed changes

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

python/cudf/cudf/core/dataframe.py Outdated Show resolved Hide resolved

Apply suggestions from code review

22b850d

- Minor style and doc update. Co-authored-by: GALI PREM SAGAR <[email protected]>

galipremsagar approved these changes Sep 23, 2020

View reviewed changes

kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer labels Sep 23, 2020

isVoid added 3 commits September 22, 2020 20:44

Merge branch 'branch-0.16' of https://github.com/rapidsai/cudf into i…

093a344

…6198

Fix a corner case, update tests

7eb0e77

When using RangeIndex, it should not be instantiated unless needed. PR 6231

Merge branch 'i6198' of https://github.com/isvoid/cudf into i6198

1262e42

kkraus14 approved these changes Sep 23, 2020

View reviewed changes

kkraus14 merged commit 2088b42 into rapidsai:branch-0.16 Sep 23, 2020

[REVIEW] Match DataFrame.set_index with pandas #6231

[REVIEW] Match DataFrame.set_index with pandas #6231

Uh oh!

Conversation

isVoid commented Sep 14, 2020

Uh oh!

GPUtester commented Sep 14, 2020

Uh oh!

codecov bot commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

isVoid commented Sep 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galipremsagar commented Sep 18, 2020

Uh oh!

galipremsagar commented Sep 18, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kkraus14 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kkraus14 commented Sep 22, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

isVoid commented Sep 23, 2020

Uh oh!

kkraus14 commented Sep 23, 2020

Uh oh!

galipremsagar commented Sep 23, 2020

Uh oh!

galipremsagar commented Sep 23, 2020

Uh oh!

Uh oh!

[REVIEW] Match `DataFrame.set_index` with pandas #6231

[REVIEW] Match `DataFrame.set_index` with pandas #6231

codecov bot commented Sep 17, 2020 •

edited

Loading

isVoid commented Sep 18, 2020 •

edited

Loading