NVIDIA / NVIDIA/cudf

[FEA] Support additional pandas features in strings wrap implementation

Open
#4,348 7 comments 0 reactions 0 assignees View on GitHub
feature request Python strings
Dominant language
C++
Stars
9.8k
Forks
1.1k
Avg merge
3d 6m
Merged PRs (30d)
278

Description

**Describe the bug**
The current libcudf implementation & old nvstrings implementation of wrap are not in-line with that of pandas `.str.wrap`: https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Series.str.wrap.html#pandas.Series.str.wrap
Our implementation seems to close to R's stringr library str_wrap function and to achieve that in Pandas we need to fix to a specific parameter setting like as follows:
```
expand_tabs = False

replace_whitespace = True

drop_whitespace = True

break_long_words = False

break_on_hyphens = False
```

There is inconsistency even if we compare with above setting of pandas to our present implementation, we seem to be maintaining tab spaces. Simple code example below.

**Steps/Code to reproduce bug**
```python
data = [' ', '\t\r\n ', ''], width = 100

def test_string_wrap(data, width):
gs = cudf.Series(data)
ps = pd.Series(data)

> assert_eq(gs.str.wrap(width=width), ps.str.wrap(width=width, break_long_words=False, expand_tabs=False, replace_whitespace=True, drop_whitespace=True, break_on_hyphens=False))

python/cudf/cudf/tests/test_string.py:1145:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pandas/_libs/testing.pyx:65: in pandas._libs.testing.assert_almost_equal
???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

> ???
E AssertionError: Series are different
E
E Series values are different (66.66667 %)
E [left]: [ , , ]
E [right]: [, , ]

```

This behavior seems to be for any value of `width`, not just 100.

If we ignore the specific parameter setting mentioned above, out of the box this is the result inconsistency between cudf and pandas implementation:
```python

data = ['line to be wrapped', 'another line to be wrapped'], width = 1

def test_string_wrap(data, width):
gs = cudf.Series(data)
ps = pd.Series(data)

> assert_eq(gs.str.wrap(width=width),ps.str.wrap(width=width))

python/cudf/cudf/tests/test_string.py:1148:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pandas/_libs/testing.pyx:65: in pandas._libs.testing.assert_almost_equal
???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

> ???
E AssertionError: Series are different
E
E Series values are different (100.0 %)
E [left]: [line
E to
E be
E wrapped, another
E line
E to
E be
E wrapped]
E [right]: [l
E i
E n
E e
E t
E o
E b
E e
E w
E r
E a
E p
E p
E e
E d, a
E n
E o
E t
E h
E e
E r
E l
E i
E n
E e
E t
E o
E b
E e
E w
E r
E a
E p
E p
E e
E d]

pandas/_libs/testing.pyx:178: AssertionError
```

**Expected behavior**
1. The values returned should be similar to that of the above.
2. I'd also like to make this as a feature request to support the currently un-supported parameters:
```
expand_tabs
replace_whitespace
drop_whitespace
break_long_words
break_on_hyphens
```

**Environment overview (please complete the following information)**
- Environment location: Docker
- Method of cuDF install: from source

**Environment details**
Output of the `cudf/print_env.sh` script here:
[env.txt](https://github.com/rapidsai/cudf/files/4295897/env.txt)

**Additional context**
Surfaced when testing this PR: #4339

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.