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

Encode values from python callback for C++ #10103

Merged
merged 3 commits into from
Jan 24, 2022
Merged

Encode values from python callback for C++ #10103

merged 3 commits into from
Jan 24, 2022

Conversation

jdye64
Copy link
Contributor

@jdye64 jdye64 commented Jan 21, 2022

Bug: This is currently throwing exceptions due to cython failing to encode the responses from the python callback automatically.

@jdye64 jdye64 requested a review from a team as a code owner January 21, 2022 16:20
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jan 21, 2022
@jdye64
Copy link
Contributor Author

jdye64 commented Jan 21, 2022

I do not have permissions to add labels but this is a non-breaking change

@galipremsagar galipremsagar added non-breaking Non-breaking change bug Something isn't working labels Jan 21, 2022
@galipremsagar
Copy link
Contributor

@jdye64 Is it required to go into 22.02?

@jdye64
Copy link
Contributor Author

jdye64 commented Jan 21, 2022 •
edited
Loading

@jdye64 Is it required to go into 22.02?

@galipremsagar No, 22.04 is fine

brandon-b-miller
brandon-b-miller approved these changes Jan 21, 2022
View reviewed changes
Copy link
Contributor

@brandon-b-miller brandon-b-miller left a comment

Choose a reason for hiding this comment

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

Cython lgtm.

@codecov
Copy link

codecov bot commented Jan 22, 2022 •
edited
Loading

Codecov Report

Merging #10103 (6edee83) into branch-22.04 (e24fa8f) will increase coverage by 0.04%.
The diff coverage is 3.18%.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.04   #10103      +/-   ##
================================================
+ Coverage         10.37%   10.42%   +0.04%     
================================================
  Files               119      119              
  Lines             20149    20607     +458     
================================================
+ Hits               2091     2148      +57     
- Misses            18058    18459     +401
Impacted Files Coverage Δ
python/cudf/cudf/__init__.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/io.py 0.00% <0.00%> (ø)
python/cudf/cudf/_fuzz_testing/orc.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/parquet.py 0.00% <ø> (ø)
python/cudf/cudf/_fuzz_testing/utils.py 0.00% <0.00%> (ø)
python/cudf/cudf/api/types.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/_base_index.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/__init__.py 0.00% <ø> (ø)
python/cudf/cudf/core/column/column.py 0.00% <0.00%> (ø)
python/cudf/cudf/core/column/decimal.py 0.00% <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 f1e0bb6...6edee83. Read the comment docs.

jsmaupin
jsmaupin approved these changes Jan 24, 2022
View reviewed changes
Copy link

@jsmaupin jsmaupin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It seems I need to approve because I left a comment? Approved, LGTM.

@galipremsagar galipremsagar removed the request for review from marlenezw January 24, 2022 16:28
@galipremsagar galipremsagar added the 5 - Ready to Merge Testing and reviews complete, ready to merge label Jan 24, 2022
@galipremsagar
Copy link
Contributor

@gpucibot merge

@rapids-bot rapids-bot bot merged commit cfb6cbe into rapidsai:branch-22.04 Jan 24, 2022
@jdye64 jdye64 deleted the custreamz_string_encoding branch January 24, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandon-b-miller brandon-b-miller brandon-b-miller approved these changes

@jsmaupin jsmaupin jsmaupin approved these changes

@galipremsagar galipremsagar galipremsagar approved these changes

Assignees
No one assigned
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.
4 participants
@jdye64 @galipremsagar @jsmaupin @brandon-b-miller