Skip to content

[REVIEW] Reindex in DataFrame.__setitem__ - #7948

Merged
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-0.20from
galipremsagar:pandas_upgrade
Apr 13, 2021
Merged

[REVIEW] Reindex in DataFrame.__setitem__#7948
rapids-bot[bot] merged 2 commits into
NVIDIA:branch-0.20from
galipremsagar:pandas_upgrade

Conversation

@galipremsagar

@galipremsagar galipremsagar commented Apr 13, 2021

Copy link
Copy Markdown
Contributor

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.

@galipremsagar galipremsagar added bug Something isn't working 3 - Ready for Review Ready for review by team Python Affects Python cuDF API. 4 - Needs cuDF (Python) Reviewer non-breaking Non-breaking change labels Apr 13, 2021
@galipremsagar galipremsagar self-assigned this Apr 13, 2021
@galipremsagar
galipremsagar requested a review from a team as a code owner April 13, 2021 17:55
@davidwendt

Copy link
Copy Markdown
Contributor

@galipremsagar

Copy link
Copy Markdown
Contributor Author

Does this change fix the following build error perchance?
https://gpuci.gpuopenanalytics.com/job/rapidsai/job/gpuci-v0.20/job/cudf/job/prb/job/cudf-gpu-test/CUDA=11.2,GPU_LABEL=gpu,OS=ubuntu18.04,PYTHON=3.8/132/testReport/

Yes, CI is currently blocked on this test failure and this PR addresses it.

@codecov

codecov Bot commented Apr 13, 2021

Copy link
Copy Markdown

Codecov Report

Merging #7948 (866c3ac) into branch-0.20 (599f62d) will increase coverage by 0.42%.
The diff coverage is 88.69%.

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

@@               Coverage Diff               @@
##           branch-0.20    #7948      +/-   ##
===============================================
+ Coverage        82.30%   82.72%   +0.42%     
===============================================
  Files              101      103       +2     
  Lines            17053    17714     +661     
===============================================
+ Hits             14035    14654     +619     
- Misses            3018     3060      +42     
Impacted Files Coverage Δ
python/cudf/cudf/utils/utils.py 83.25% <ø> (-1.81%) ⬇️
python/dask_cudf/dask_cudf/backends.py 89.58% <ø> (-0.05%) ⬇️
python/cudf/cudf/utils/dtypes.py 83.44% <46.66%> (-6.45%) ⬇️
python/cudf/cudf/core/groupby/groupby.py 92.41% <78.57%> (-1.04%) ⬇️
python/cudf/cudf/core/column/lists.py 87.41% <80.00%> (+0.19%) ⬆️
python/cudf/cudf/core/column/struct.py 96.29% <86.66%> (-3.71%) ⬇️
python/cudf/cudf/core/index.py 93.04% <88.09%> (+0.01%) ⬆️
python/cudf/cudf/core/column/column.py 87.86% <88.57%> (+0.43%) ⬆️
python/cudf/cudf/core/column/decimal.py 92.92% <91.48%> (-0.92%) ⬇️
python/cudf/cudf/core/column/interval.py 91.11% <92.30%> (+0.48%) ⬆️
... and 66 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 18964ff...efa0ec2. Read the comment docs.

@galipremsagar galipremsagar 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 Apr 13, 2021
@galipremsagar

Copy link
Copy Markdown
Contributor Author

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 8dc1559 into NVIDIA:branch-0.20 Apr 13, 2021
galipremsagar added a commit to galipremsagar/cudf that referenced this pull request Apr 14, 2021
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: NVIDIA#7948
kkraus14 pushed a commit that referenced this pull request Apr 16, 2021
* Reindex in `DataFrame.__setitem__` (#7948)

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: #7948

* pin max pandas to 1.2.4
raydouglass pushed a commit that referenced this pull request Apr 22, 2021
…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

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

Development

Successfully merging this pull request may close these issues.

3 participants