Skip to content

Enable returning string data from UDFs used through apply - #11933

Merged
rapids-bot[bot] merged 55 commits into
NVIDIA:branch-22.12from
brandon-b-miller:fea-strings-udf-return-strings
Nov 8, 2022
Merged

Enable returning string data from UDFs used through apply#11933
rapids-bot[bot] merged 55 commits into
NVIDIA:branch-22.12from
brandon-b-miller:fea-strings-udf-return-strings

Conversation

@brandon-b-miller

@brandon-b-miller brandon-b-miller commented Oct 17, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR introduces the ability to return a string from a UDF used through DataFrame.apply or Series.apply. It provides all of the plumbing needed to run the function lambda st: return st, but does not provide any APIs that return strings such as strip or upper - these will be added in a series of followups. A cast from string_view to udf_string is provided that numba will call when attempting to return a string_view into a udf_string array.

@brandon-b-miller brandon-b-miller added feature request New feature or request 2 - In Progress Currently a work in progress numba Numba issue Python Affects Python cuDF API. non-breaking Non-breaking change labels Oct 17, 2022
@brandon-b-miller
brandon-b-miller requested a review from a team as a code owner October 17, 2022 18:24
@codecov

codecov Bot commented Oct 17, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Base: 87.47% // Head: 88.10% // Increases project coverage by +0.62% 🎉

Coverage data is based on head (26784be) compared to base (f817d96).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@               Coverage Diff                @@
##           branch-22.12   #11933      +/-   ##
================================================
+ Coverage         87.47%   88.10%   +0.62%     
================================================
  Files               133      135       +2     
  Lines             21826    22057     +231     
================================================
+ Hits              19093    19433     +340     
+ Misses             2733     2624     -109
Impacted Files Coverage Δ
python/cudf/cudf/core/column/interval.py 85.45% <0.00%> (-9.10%) ⬇️
python/cudf/cudf/io/text.py 91.66% <0.00%> (-8.34%) ⬇️
python/cudf/cudf/core/_base_index.py 81.28% <0.00%> (-4.27%) ⬇️
python/cudf/cudf/io/json.py 92.06% <0.00%> (-2.68%) ⬇️
python/cudf/cudf/utils/utils.py 89.91% <0.00%> (-0.69%) ⬇️
python/cudf/cudf/core/column/timedelta.py 90.17% <0.00%> (-0.58%) ⬇️
python/cudf/cudf/core/column/datetime.py 89.21% <0.00%> (-0.51%) ⬇️
python/cudf/cudf/core/column/column.py 87.96% <0.00%> (-0.46%) ⬇️
python/dask_cudf/dask_cudf/core.py 73.72% <0.00%> (-0.41%) ⬇️
python/cudf/cudf/io/parquet.py 90.45% <0.00%> (-0.39%) ⬇️
... and 42 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@vyasr

vyasr commented Oct 18, 2022

Copy link
Copy Markdown
Contributor

@brandon-b-miller I'm going to convert this to a draft until the upstream PR is merged.

@vyasr
vyasr marked this pull request as draft October 18, 2022 17:35
@brandon-b-miller brandon-b-miller mentioned this pull request Oct 19, 2022
Merged
3 tasks
shwina
shwina reviewed Oct 19, 2022
View reviewed changes
Comment thread python/cudf/cudf/core/udf/utils.py Outdated
@davidwendt

davidwendt commented Nov 1, 2022

Copy link
Copy Markdown
Contributor

One significant change in #11912 is that the column_from_udf_string_array no longer automatically frees the udf_string objects.
You need to call the new free_udf_string_array after column_from_udf_string_array in order to free them.

@brandon-b-miller

brandon-b-miller commented Nov 1, 2022

Copy link
Copy Markdown
Contributor Author

One significant change in #11912 is that the column_from_udf_string_array no longer automatically frees the udf_string objects. You need to call the new free_udf_string_array after column_from_udf_string_array in order to free them.

got it- going to merge the latest from #11912 now and work towards passing tests here.

@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Nov 1, 2022
@github-actions github-actions Bot removed the libcudf Affects libcudf (C++/CUDA) code. label Nov 2, 2022
@brandon-b-miller
brandon-b-miller marked this pull request as ready for review November 2, 2022 14:13
davidwendt
davidwendt reviewed Nov 2, 2022
View reviewed changes
Comment thread python/strings_udf/strings_udf/_lib/cudf_jit_udf.pyx Outdated
davidwendt
davidwendt reviewed Nov 2, 2022
View reviewed changes
Comment thread python/strings_udf/strings_udf/_lib/cpp/strings_udf.pxd Outdated
shwina
shwina reviewed Nov 2, 2022
View reviewed changes
Comment thread python/strings_udf/strings_udf/_lib/cudf_jit_udf.pyx Outdated
@brandon-b-miller brandon-b-miller added 3 - Ready for Review Ready for review by team and removed 2 - In Progress Currently a work in progress labels Nov 3, 2022
vyasr
vyasr requested changes Nov 4, 2022
View reviewed changes

@vyasr vyasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I love the small changeset! I only have small suggestions, I don't think there's any major changes to make.

Comment thread python/strings_udf/strings_udf/_lib/cpp/strings_udf.pxd
Comment thread python/strings_udf/strings_udf/_lib/cudf_jit_udf.pyx Outdated
Comment thread python/strings_udf/strings_udf/_lib/cpp/strings_udf.pxd Outdated
Comment thread python/strings_udf/strings_udf/_typing.py
Comment thread python/strings_udf/strings_udf/_typing.py
Comment thread python/strings_udf/strings_udf/tests/test_string_udfs.py Outdated
Comment thread python/strings_udf/strings_udf/lowering.py
Comment thread python/cudf/cudf/core/udf/utils.py Outdated
Comment thread python/cudf/cudf/core/udf/utils.py Outdated
vyasr
vyasr approved these changes Nov 7, 2022
View reviewed changes

@vyasr vyasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

bdice
bdice approved these changes Nov 7, 2022
View reviewed changes

@bdice bdice left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All looks good to me.

@bdice

bdice commented Nov 7, 2022

Copy link
Copy Markdown
Contributor

@brandon-b-miller Before merging, can you write a description for this PR?

@brandon-b-miller

brandon-b-miller commented Nov 7, 2022

Copy link
Copy Markdown
Contributor Author

rerun tests

@brandon-b-miller

brandon-b-miller commented Nov 8, 2022

Copy link
Copy Markdown
Contributor Author

@gpucibot merge

@rapids-bot
rapids-bot Bot merged commit 35077f5 into NVIDIA:branch-22.12 Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davidwendt davidwendt davidwendt left review comments
@vyasr vyasr vyasr approved these changes
@bdice bdice bdice approved these changes
@galipremsagar galipremsagar Awaiting requested review from galipremsagar galipremsagar was automatically assigned from rapidsai/cudf-python-codeowners
+1 more reviewer
@shwina shwina shwina left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

3 - Ready for Review Ready for review by team feature request New feature or request non-breaking Non-breaking change numba Numba issue Python Affects Python cuDF API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@brandon-b-miller @vyasr @davidwendt @bdice @shwina