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

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

Merged
merged 4 commits into from
Aug 3, 2021
Merged

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

merged 4 commits into from
Aug 3, 2021

Conversation

NV-jpt
Copy link
Contributor

@NV-jpt NV-jpt commented Jul 28, 2021 •
edited
Loading

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 •
edited
Loading

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!

shwina
shwina approved these changes Jul 30, 2021
View reviewed changes
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

@NV-jpt NV-jpt mentioned this pull request Aug 18, 2021
Merged
@beckernick beckernick mentioned this pull request Aug 30, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@beckernick beckernick beckernick left review comments

@shwina shwina shwina approved these changes

@marlenezw marlenezw Awaiting requested review from marlenezw

@skirui-source skirui-source Awaiting requested review from skirui-source

Assignees
No one assigned
Labels
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
@NV-jpt @shwina @beckernick