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

[REVIEW] Reindex in DataFrame.__setitem__ #7957

Merged
merged 2 commits into from
Apr 16, 2021
Merged

[REVIEW] Reindex in DataFrame.__setitem__ #7957

merged 2 commits into from
Apr 16, 2021

Conversation

galipremsagar
Copy link
Contributor

Backporting a fix from 0.20: 8dc1559

Also pinning max pandas version to 1.2.4.

@galipremsagar
Reindex in DataFrame.__setitem__ (rapidsai#7948)
f451fd6 
This PR fixes missing reindexing in `DataFrame.__setitem__` when the `value` argument is a `DataFrame`, we currently align index if `value` is a Series & `arg` is a column name already.

This change is necessary to continue with the upgrade to pandas `1.2.4`,  however pandas has confirmed this as a regression only in `1.2.0` and `1.2.2` hence corrected the pytest to only xfail in those versions of pandas.

Authors:
  - GALI PREM SAGAR (https://github.com/galipremsagar)

Approvers:
  - Ashwin Srinath (https://github.com/shwina)

URL: rapidsai#7948
@galipremsagar
pin max pandas to 1.2.4
f1c48cd
@galipremsagar galipremsagar added bug Something isn't working 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer ! - Hotfix Hotfix is a bug that affects the majority of users for which there is no reasonable workaround non-breaking Non-breaking change labels Apr 14, 2021
@galipremsagar galipremsagar requested a review from kkraus14 April 14, 2021 19:44
@galipremsagar galipremsagar self-assigned this Apr 14, 2021
@galipremsagar galipremsagar requested a review from a team as a code owner April 14, 2021 19:44
@galipremsagar galipremsagar requested review from a team as code owners April 14, 2021 19:44
@galipremsagar galipremsagar requested review from shwina and removed request for a team April 14, 2021 19:44
@github-actions github-actions bot added conda Python Affects Python cuDF API. labels Apr 14, 2021
@kkraus14 kkraus14 added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed ! - Hotfix Hotfix is a bug that affects the majority of users for which there is no reasonable workaround 3 - Ready for Review Ready for review by team 4 - Needs cuDF (Python) Reviewer labels Apr 14, 2021
@galipremsagar
Copy link
Contributor Author

rerun tests

@kkraus14 kkraus14 changed the title [REVIEW][HOTFIX] Reindex in DataFrame.__setitem__ [REVIEW] Reindex in DataFrame.__setitem__ Apr 15, 2021
@codecov
Copy link

codecov bot commented Apr 16, 2021 •
edited
Loading

Codecov Report

Merging #7957 (f439b2d) into branch-0.19 (a5d2407) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head f439b2d differs from pull request most recent head f1c48cd. Consider uploading reports for the commit f1c48cd to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##           branch-0.19    #7957   +/-   ##
============================================
  Coverage        82.74%   82.74%           
============================================
  Files              103      103           
  Lines            17701    17703    +2     
============================================
+ Hits             14647    14649    +2     
  Misses            3054     3054
Impacted Files Coverage Δ
python/cudf/cudf/core/_compat.py 100.00% <100.00%> (ø)
python/cudf/cudf/core/dataframe.py 90.86% <100.00%> (+<0.01%) ⬆️

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 a5d2407...f1c48cd. Read the comment docs.

@kkraus14
Copy link
Collaborator

@gpucibot merge

@kkraus14 kkraus14 merged commit 8e1ffd4 into rapidsai:branch-0.19 Apr 16, 2021
@vyasr vyasr mentioned this pull request Apr 21, 2021
Merged
raydouglass pushed a commit that referenced this pull request Apr 22, 2021
@vyasr
Don't reindex an new value on setitem if the original dataframe was e…
86897fb 
…mpty (#8026)

This PR resolves #8023. The pandas compatibility patch in #7948 (and the backport to 0.19 in #7957) also needs to handle the case of assigning to an empty DataFrame.

Url: #8026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dillon-cullinan dillon-cullinan dillon-cullinan approved these changes

@kkraus14 kkraus14 kkraus14 approved these changes

@shwina shwina Awaiting requested review from shwina

Assignees

@galipremsagar galipremsagar

Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@galipremsagar @kkraus14 @dillon-cullinan