Add format API for list column of strings #9454

davidwendt · 2021-10-15T18:52:25Z

This PR adds API cudf::strings::format_list_column to create the formatted output as described in #8351. The API only accepts lists columns of strings.

Example 1
  l1 = { [[a,b,c], [d,e]], [[f,g], [h]] }
  s1 = format_list_column(l1)
  s1 is now ["[[a,b,c],[d,e]]", "[[f,g],[h]]"]

Example 2
  l2 = { [[a,b,c], [d,e]], [NULL], [[f,g], NULL [h]] }
  s2 = format_list_column(l1, '-', [':', '{', '}'])
  s2 is now ["{{a:b:c}:{d:e}}", "{-}", "{{f:g}:-:{h}}"]

The format API takes parameters to specify the strings to use for [ , ] and ',' as well as the string used to represent null entries.

codecov · 2021-10-15T20:08:52Z

Codecov Report

Merging #9454 (bfb4118) into branch-21.12 (ab4bfaa) will decrease coverage by 0.15%.
The diff coverage is n/a.

❗ Current head bfb4118 differs from pull request most recent head e9dbe8d. Consider uploading reports for the commit e9dbe8d to get more accurate results

@@               Coverage Diff                @@
##           branch-21.12    #9454      +/-   ##
================================================
- Coverage         10.79%   10.63%   -0.16%     
================================================
  Files               116      117       +1     
  Lines             18869    19356     +487     
================================================
+ Hits               2036     2059      +23     
- Misses            16833    17297     +464

Impacted Files	Coverage Δ
python/dask_cudf/dask_cudf/sorting.py	`92.90% <0.00%> (-1.21%)`	⬇️
python/cudf/cudf/io/csv.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/io/orc.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/__init__.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/core/frame.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/core/index.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/io/parquet.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/core/series.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/core/reshape.py	`0.00% <0.00%> (ø)`
python/cudf/cudf/utils/dtypes.py	`0.00% <0.00%> (ø)`
... and 42 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 f041a47...e9dbe8d. Read the comment docs.

python/cudf/cudf/core/column/lists.py

robertmaynard

CMake changes LGTM

ajschmidt8

Approving ops-codeowner file changes

vyasr · 2021-10-28T18:44:09Z

cpp/include/cudf/strings/convert/convert_lists.hpp

+  strings_column_view const& separators = strings_column_view(column_view{
+    data_type{type_id::STRING}, 0, nullptr}),


Is there a reason to prefer a column rather than three separate scalars? Admittedly that adds more parameters to the API, but it seems awkward to stuff all three in a column (especially since I would anticipate that overloading the element separator would be a largely independent request from overriding the enclosures).

It is a little more efficient to create a column of strings which normally is a single device copy rather than individual scalars which would be 3 small device copies.

That's true, but it feels like a (minor) abuse of a column to stuff in three values that are semantically different but happen to be of the same type. The elements of a column seem like they should all "mean" the same thing, if that makes sense. This feels like a premature optimization, but H2D copies are expensive so maybe the improvement is worth it. I trust your judgment there, just felt odd to me.

cpp/src/strings/convert/convert_lists.cu

vyasr

I left a few open questions, but not much clearly actionable and mostly just to satisfy my curiosity. I think we can ship it.

cpp/src/strings/convert/convert_lists.cu

python/cudf/cudf/core/column/lists.py

karthikeyann

LGTM 👍

karthikeyann · 2021-11-09T14:47:23Z

cpp/tests/strings/format_lists_tests.cpp

+
+TEST_F(StringsFormatListsTest, EmptyList)
+{
+  using STR_LISTS = cudf::test::lists_column_wrapper<cudf::string_view>;


This repeated line could be moved to the top.

davidwendt · 2021-11-09T23:05:41Z

@gpucibot merge

Add format API for list column of strings

f79ef82

davidwendt added feature request New feature or request 2 - In Progress Currently a work in progress libcudf Affects libcudf (C++/CUDA) code. strings strings issues (C++ and Python) non-breaking Non-breaking change labels Oct 15, 2021

davidwendt self-assigned this Oct 15, 2021

github-actions bot added the CMake CMake build issue label Oct 15, 2021

add python/cython for format_list_column

cd24c76

github-actions bot added the Python Affects Python cuDF API. label Oct 15, 2021

davidwendt added 4 commits October 17, 2021 18:03

fix style check error

ccebd72

update comments

64500f7

support nested lists in as_string_column

3f4c2d1

add pytest for list.as_string_column

96c4327

shwina reviewed Oct 18, 2021

View reviewed changes

python/cudf/cudf/core/column/lists.py Outdated Show resolved Hide resolved

shwina reviewed Oct 18, 2021

View reviewed changes

python/cudf/cudf/core/column/lists.py Show resolved Hide resolved

shwina reviewed Oct 18, 2021

View reviewed changes

python/cudf/cudf/core/column/lists.py Outdated Show resolved Hide resolved

davidwendt added 6 commits October 18, 2021 15:13

fix mypy style check error

23c4013

fix mypy style check error 2

1ee7005

fix mypy style check error 3

9e08148

add convert_lists.hpp to meta.yaml

e62ca31

add pytest data with floats

17c1b37

fix typo in doxygen comment

a6f8793

github-actions bot added the conda label Oct 19, 2021

davidwendt added 4 commits October 19, 2021 11:28

fix comments, spacing

5869ffd

Merge branch 'branch-21.12' into fea-strings-format-lists

e293ad0

Merge branch 'branch-21.12' into fea-strings-format-lists

223885f

fix doxygen comments, typos

8ca60f9

davidwendt marked this pull request as ready for review October 20, 2021 19:17

davidwendt requested review from a team as code owners October 20, 2021 19:17

davidwendt requested review from robertmaynard, karthikeyann, vyasr and marlenezw October 20, 2021 19:17

davidwendt added 3 commits October 21, 2021 09:50

fix merge conflict

55cc488

name hardcoded separator index

7fcccce

reformat some comments

7c6320f

robertmaynard approved these changes Oct 25, 2021

View reviewed changes

ajschmidt8 approved these changes Oct 25, 2021

View reviewed changes

Merge branch 'branch-21.12' into fea-strings-format-lists

484146c

vyasr requested changes Oct 28, 2021

View reviewed changes

davidwendt requested a review from vyasr October 29, 2021 11:55

davidwendt added 2 commits November 1, 2021 16:31

change depth++ to ++depth

6a1da5f

Merge branch 'branch-21.12' into fea-strings-format-lists

4e387d1

vyasr approved these changes Nov 1, 2021

View reviewed changes

cpp/src/strings/convert/convert_lists.cu Show resolved Hide resolved

vyasr reviewed Nov 1, 2021

View reviewed changes

python/cudf/cudf/core/column/lists.py Show resolved Hide resolved

davidwendt added 3 commits November 1, 2021 19:45

item-separator inherit from int8

5a2ed2b

fix merge conflicts

96b07d5

Merge branch 'branch-21.12' into fea-strings-format-lists

e9dbe8d

karthikeyann approved these changes Nov 9, 2021

View reviewed changes

rapids-bot bot merged commit 499ebae into rapidsai:branch-21.12 Nov 9, 2021

davidwendt deleted the fea-strings-format-lists branch November 10, 2021 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add format API for list column of strings #9454

Add format API for list column of strings #9454

davidwendt commented Oct 15, 2021 •

edited

Loading

codecov bot commented Oct 15, 2021 •

edited

Loading

robertmaynard left a comment

ajschmidt8 left a comment

vyasr Oct 28, 2021

davidwendt Oct 28, 2021

vyasr Nov 1, 2021

vyasr left a comment

karthikeyann left a comment

karthikeyann Nov 9, 2021

davidwendt commented Nov 9, 2021

		strings_column_view const& separators = strings_column_view(column_view{
		data_type{type_id::STRING}, 0, nullptr}),

Add format API for list column of strings #9454

Add format API for list column of strings #9454

Conversation

davidwendt commented Oct 15, 2021 • edited Loading

codecov bot commented Oct 15, 2021 • edited Loading

Codecov Report

robertmaynard left a comment

Choose a reason for hiding this comment

ajschmidt8 left a comment

Choose a reason for hiding this comment

vyasr Oct 28, 2021

Choose a reason for hiding this comment

davidwendt Oct 28, 2021

Choose a reason for hiding this comment

vyasr Nov 1, 2021

Choose a reason for hiding this comment

vyasr left a comment

Choose a reason for hiding this comment

karthikeyann left a comment

Choose a reason for hiding this comment

karthikeyann Nov 9, 2021

Choose a reason for hiding this comment

davidwendt commented Nov 9, 2021

davidwendt commented Oct 15, 2021 •

edited

Loading

codecov bot commented Oct 15, 2021 •

edited

Loading