ENH: Implement core/strings/wrap method #6705

jeffreystarr · 2014-03-25T19:15:47Z

This patch implements the str_wrap function within core/strings. The implementation follows
the behavior of R's stringr library. When a string is 'wrap'ped, the return value will be a
paragraph with lines of max(word length, width) length.

Example:

>>> str_wrap(Series([u'line to be wrapped', u'another line to be wrapped']), width=12)
Series([u'line to be\nwrapped', u'another\nline to be\nwrapped'])

…ary)

This patch implements the str_wrap function within core/strings. The implementation follows the behavior of R's stringr library. When a string is 'wrap'ped, the return value will be a paragraph with lines of max(word length, width) length.

jreback · 2014-03-25T19:22:24Z

can you show an example in the top fo the PR of using this.

Also need to add to the documentation in basics.rst (mention this function, don't need an example)

also add to comparison_with_r.rst

dsm054 · 2014-03-25T19:33:14Z

Couldn't this leverage the textwrap module?

…R's stringr library defaults and semantics (width being exclusive).

Expanded docstring with description of major optional parameters and added an example.

jeffreystarr · 2014-03-26T19:04:02Z

I've changed the implementation to use the textwrap module and expanded the documentation to reference many of the parameters wrap can now use.

I've also added a mention of wrap to basics.rst.

comparison_with_r.rst does not currently discuss any vectorized string operations. I think a new section to that file (to discuss all vectorized string operations) would be a PR by itself.

jreback · 2014-03-26T19:06:49Z

pandas/core/strings.py

@@ -710,20 +711,64 @@ def str_rstrip(arr, to_strip=None):
    return _na_map(lambda x: x.rstrip(to_strip), arr)


-def str_wrap(arr, width=80):
+def str_wrap(arr, **kwargs):


should all of the arguments and their defaults be here? is their a reason you did it this other way?

Since str_wrap is a wrapper around textwrap.TextWrapper.wrap, I did not want to restrict the interface to a subset of the options available. (There have been at least three parameters added to textwrap.TextWrapper from python 2.6 to 3.3.)

hmm...that makes sense, the only issue then is the doc string.... @jorisvandenbossche any idea?

@jeffreystarr does the doc-string work?

I think it is OK now, with the docstring explaining the parameters. It's only the signature now that does not include all those.
But what is the reason not including the keyword arguments and its default values in the signature but instead in a dict in the function?

jorisvandenbossche · 2014-03-26T23:07:02Z

pandas/core/strings.py

+    library str_wrap function. Unless overwritten using kwargs, the instance has expand_tabs=False,
+    replace_whitespace=True, drop_whitespace=True, break_long_words=False, and
+    break_on_hyphens=False. Since R's stringr str_wrap treats the line width as an exclusive
+    value, the instance is configured with width=user-supplied width - 1.


Shouldn't we rather go with python's default value instead of changing the user input? And then putting a note explaining the difference with R?

…Python 3.

… (thus matching textwrap module)

jreback · 2014-04-05T23:53:14Z

doc/source/release.rst

@@ -195,6 +195,7 @@ Improvements to existing features
 - Performance improvement when converting ``DatetimeIndex`` to floating ordinals
  using ``DatetimeConverter`` (:issue:`6636`)
 - Performance improvement for  ``DataFrame.shift`` (:issue: `5609`)
+- Arrays of strings can be wrapped to a specified width (str.wrap)


can you add the refernce to this PR number

jreback · 2014-04-05T23:53:25Z

@jeffreystarr pls rebase this

jtratner · 2014-04-06T00:41:38Z

@jeffreystarr - I'd also like to make width a positional argument, which lets you do something like: df.str.wrap(30), which has a nice clarity to it vs. df.str.wrap(width=30). I'd suggest width should be required, because I'd say there isn't really a "default" width that really makes sense. This is a pretty minimal change, just needs to be def str_wrap(self, width, **kwargs) in each function.

As an aside, this previously was just NotImplemented right? So there's no issue with forcing width to be passed?

jeffreystarr · 2014-04-06T00:47:25Z

You are correct - it was previously NotImplemented. There is no impact to
existing code by making width required.

On Sat, Apr 5, 2014 at 7:41 PM, Jeff Tratner [email protected]:

@jeffreystarr https://github.com/jeffreystarr - I'd also like to make
width a positional argument, which lets you do something like:
df.str.wrap(30), which has a nice clarity to it vs. df.str.wrap(width=30).
I'd suggest width should be required, because I'd say there isn't really
a "default" width that really makes sense. This is a pretty minimal change,
just needs to be def str_wrap(self, width, **kwargs) in each function.

As an aside, this previously was just NotImplemented right? So there's no
issue with forcing width to be passed?

Reply to this email directly or view it on GitHubhttps://github.com//pull/6705#issuecomment-39655369
.

jtratner · 2014-04-06T00:58:52Z

@jeffreystarr - great, then if it's okay with you, I'd suggest changing width to be required.

…rd argument)

jreback · 2014-04-08T23:57:56Z

@jeffreystarr this looks good

can you can squash down to 1 commit would be great

jorisvandenbossche · 2014-04-09T08:39:53Z

pandas/core/strings.py

    width : int
        Maximum line-width
+    expand_tabs: bool, optional


There should be a space before the : (like for width above)

… are convertible

…pes (GH6982)

Add test case for iterator data argument Move the type check up to where other checks are performed Use asserRaisesRegexp for more specific checking Add fix to the release notes

…ndexer

jreback · 2014-04-28T23:41:41Z

hmm...looks like you somehow rebase to a different head

git rebase -i origin/master

should do the trick

if it doesn't, then:

pull master
create a new branch
git cherry-pick <commits> (you must pick 1 by one, starting with the oldest
the are YOUR commits (I think you have 6)

then this 'starts' you over

you can then

git push yourforck yourbranch -f

and it will erase this and replace with your new

jorisvandenbossche · 2014-04-29T07:31:16Z

small note on the advice of @jreback: be sure that origin (if this is your fork) is also updated, or you can rebase on upstream (if this is pydata/pandas):

git fetch upstream
git rebase upstream/master

…ary)

This patch implements the str_wrap function within core/strings. The implementation follows the behavior of R's stringr library. When a string is 'wrap'ped, the return value will be a paragraph with lines of max(word length, width) length.

…R's stringr library defaults and semantics (width being exclusive).

Expanded docstring with description of major optional parameters and added an example.

…Python 3.

… (thus matching textwrap module)

…rd argument)

…an the direct str_wrap form.

…p defaults

…o str_wrap Conflicts: doc/source/release.rst doc/source/v0.14.0.txt

jreback · 2014-04-29T16:29:53Z

closing in favor of #6999

jeffreystarr added 3 commits March 25, 2014 13:40

Implementing str_wrap function (matching behavior to R's stringr libr…

39fe4f8

…ary)

ENH: Implement core/strings/wrap method

d7ac7e8

This patch implements the str_wrap function within core/strings. The implementation follows the behavior of R's stringr library. When a string is 'wrap'ped, the return value will be a paragraph with lines of max(word length, width) length.

str.wrap added to release notes

76f4c18

jreback added Strings labels Mar 25, 2014

jreback added this to the 0.14.0 milestone Mar 25, 2014

jeffreystarr added 3 commits March 26, 2014 13:29

Changed implementation to leverage textwrap module, but still retain …

41d603b

…R's stringr library defaults and semantics (width being exclusive).

Fixed width default to match documentation.

5ef37ec

Expanded docstring with description of major optional parameters and added an example.

Added wrap method to table of vectorized string methods

84d870a

jreback reviewed Mar 26, 2014
View reviewed changes

Added test for non-unicode and NaN input

6a3769f

jorisvandenbossche reviewed Mar 26, 2014
View reviewed changes

jeffreystarr added 2 commits March 27, 2014 11:14

Removed explicit unicode strings (u'') as they are invalid syntax in …

e2ff489

…Python 3.

Changed width from meaning exclusive width to meaning inclusive width…

007f2d8

… (thus matching textwrap module)

jreback reviewed Apr 5, 2014
View reviewed changes

jeffreystarr added 3 commits April 6, 2014 12:26

Added a non-ascii encodable unicode test

656b1ae

Added reference to pull request to release note

c0bd652

Made width a required, positional argument (versus an optional, keywo…

f357150

…rd argument)

jorisvandenbossche reviewed Apr 9, 2014
View reviewed changes

jreback and others added 8 commits April 28, 2014 06:59

COMPAT: fix warning for slice indexers when using floats in iloc that…

b3c927e

… are convertible

TST: test_frame/test_sum not comparing correctly on smaller sized dty…

8e31bae

…pes (GH6982)

COMPAT: remove deprecated options from printing in test suite (GH6984)

2c96614

PERF: improved performance of compatible pickles (GH6899)

44d73a4

ERR: Add check for iterators when creating DataFrame

13f8fc1

Add test case for iterator data argument Move the type check up to where other checks are performed Use asserRaisesRegexp for more specific checking Add fix to the release notes

TST: test_sum compare of less precise dtypes take 2

25821f8

CLN: drop internals._invert_reordering in favour of lib.get_reverse_i…

6d49dc3

…ndexer

str.wrap added to release notes

169e53c

jeffreystarr added 17 commits April 29, 2014 08:45

Implementing str_wrap function (matching behavior to R's stringr libr…

110c3e0

…ary)

ENH: Implement core/strings/wrap method

8469a45

This patch implements the str_wrap function within core/strings. The implementation follows the behavior of R's stringr library. When a string is 'wrap'ped, the return value will be a paragraph with lines of max(word length, width) length.

str.wrap added to release notes

03864c2

Changed implementation to leverage textwrap module, but still retain …

d338ea2

…R's stringr library defaults and semantics (width being exclusive).

Fixed width default to match documentation.

71134b9

Expanded docstring with description of major optional parameters and added an example.

Added wrap method to table of vectorized string methods

3c804ae

Added test for non-unicode and NaN input

3a66151

Removed explicit unicode strings (u'') as they are invalid syntax in …

e91e942

…Python 3.

Changed width from meaning exclusive width to meaning inclusive width…

5090e5e

… (thus matching textwrap module)

Added a non-ascii encodable unicode test

fe9c8be

Added reference to pull request to release note

2af4325

Made width a required, positional argument (versus an optional, keywo…

5cd3bdc

…rd argument)

Add spaces before colon within docstring

b31834e

Change example in docstring to use the series.str.wrap form rather th…

ae8f529

…an the direct str_wrap form.

Change code to use Python's textwrap defaults rather than R's str_wra…

331ad4b

…p defaults

str.wrap added to release notes

446b928

Merge branch 'str_wrap' of https://github.com/jeffreystarr/pandas int…

0980503

…o str_wrap Conflicts: doc/source/release.rst doc/source/v0.14.0.txt

jeffreystarr mentioned this pull request Apr 29, 2014

ENH: Implement core/strings/wrap method #6999

Closed

jreback closed this Apr 29, 2014

Uh oh!

ENH: Implement core/strings/wrap method #6705

ENH: Implement core/strings/wrap method #6705

Uh oh!

Conversation

jeffreystarr commented Mar 25, 2014

Uh oh!

jreback commented Mar 25, 2014

Uh oh!

dsm054 commented Mar 25, 2014

Uh oh!

jeffreystarr commented Mar 26, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Apr 5, 2014

Uh oh!

jtratner commented Apr 6, 2014

Uh oh!

jeffreystarr commented Apr 6, 2014

Uh oh!

jtratner commented Apr 6, 2014

Uh oh!

jreback commented Apr 8, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented Apr 28, 2014

Uh oh!

jorisvandenbossche commented Apr 29, 2014

Uh oh!

jreback commented Apr 29, 2014

Uh oh!

Uh oh!