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

Throw KeyError when accessing field from struct with nonexistent key #8880

Merged
merged 4 commits into from
Aug 3, 2021

Conversation

NV-jpt
Copy link
Contributor

@NV-jpt NV-jpt commented Jul 28, 2021

PR response to issue #8875

@NV-jpt NV-jpt requested a review from a team as a code owner July 28, 2021 15:01
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jul 28, 2021
@shwina
Copy link
Contributor

shwina commented Jul 28, 2021

Thanks for the fix! Could you also please add a test in test_struct.py for the behaviour you are adding?

@codecov
Copy link

codecov bot commented Jul 28, 2021

Codecov Report

Merging #8880 (b349d0f) into branch-21.10 (18f7c01) will decrease coverage by 0.06%.
The diff coverage is n/a.

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

@@               Coverage Diff                @@
##           branch-21.10    #8880      +/-   ##
================================================
- Coverage         10.67%   10.61%   -0.07%     
================================================
  Files               110      116       +6     
  Lines             18271    19001     +730     
================================================
+ Hits               1951     2017      +66     
- Misses            16320    16984     +664     
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <ø> (ø)
python/cudf/cudf/core/__init__.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/categorical.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/column.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/lists.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/methods.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/numerical.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/string.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/struct.py 0.00% <ø> (ø)
python/cudf/cudf/core/dataframe.py 0.00% <ø> (ø)
... and 81 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 904222b...dca6999. Read the comment docs.

@beckernick beckernick added bug Something isn't working non-breaking Non-breaking change labels Jul 28, 2021
@shwina
Copy link
Contributor

shwina commented Jul 29, 2021

Thanks! This looks great -- just need a test or two in test_struct.py and should be good to go!

Copy link
Contributor

@shwina shwina left a comment

Choose a reason for hiding this comment

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

LGTM! Nice work, @NV-jpt

@NV-jpt
Copy link
Contributor Author

NV-jpt commented Jul 30, 2021

Awesome!! Thank you for all the help @shwina & @beckernick !

@NV-jpt
Copy link
Contributor Author

NV-jpt commented Aug 3, 2021

One of the checks was not successful (gpuCI/cudf/gpu/java — Build #429 failed in 44 min).

Is there something I can do to resolve this?

@shwina
Copy link
Contributor

shwina commented Aug 3, 2021

rerun tests

@shwina
Copy link
Contributor

shwina commented Aug 3, 2021

Doesn't look like an issue with this PR. Just reran tests -- let's see if it passes this time around.

@NV-jpt
Copy link
Contributor Author

NV-jpt commented Aug 3, 2021

Thank you, @shwina! I could not figure out how to rerun them

@shwina
Copy link
Contributor

shwina commented Aug 3, 2021

Ah, I'm not sure if it's available to everyone, but the comment rerun tests (#8880 (comment)) triggers the rerun. Hopefully, this comment won't trigger a rerun...

@shwina
Copy link
Contributor

shwina commented Aug 3, 2021

Oh no, it did!

@beckernick
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit c3e71e2 into rapidsai:branch-21.10 Aug 3, 2021
@beckernick
Copy link
Member

This should unblock #8874

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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