DOC: add sep argument to read_clipboard signature #14537

keshavramaswamy · 2016-10-29T17:56:08Z

closes DOC: give read_clipboard a signature #13726

codecov-io · 2016-10-29T18:53:11Z

Current coverage is 85.26% (diff: 100%)

Merging #14537 into master will decrease coverage by <.01%

@@             master     #14537   diff @@
==========================================
  Files           140        140          
  Lines         50693      50672    -21   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          43229      43208    -21   
  Misses         7464       7464          
  Partials          0          0

Powered by Codecov. Last update e1cdc4b...18c30eb

TomAugspurger

Could verify that we have tests for all the cases of sep and delim_whitespace, and maybe add a few if we don't?

TomAugspurger · 2016-10-30T12:07:26Z

pandas/io/clipboard.py

    """
    Read text from clipboard and pass to read_table. See read_table for the
    full argument list

-    If unspecified, `sep` defaults to '\s+'
+    If unspecified, `sep` defaults to '\s+' and delim_whitespace defaults


Take a look at the NumPy docstring reference, which we try to follow. So this would be something like like

sep : str, optional A string or regex delimiter. Default `'\s+'` delim_whitespace : bool, optional Whether to .... Default False

TomAugspurger · 2016-10-30T12:08:50Z

pandas/io/clipboard.py


-    return read_table(StringIO(text), **kwargs)
+    return read_table(StringIO(text), sep, delim_whitespace, **kwargs)


Could you make these keyword arguments (sep=sep, delim_whitespace=delim_whitespace)

@TomAugspurger Thanks for your review. Will push in the required changes.

jorisvandenbossche · 2016-10-30T17:17:14Z

I would not list delim_whitespace specifically, as this is actually a equivalent for sep='\s+' (that is the reason it is checked for in the code, as it is an alternative way to specify the sep).

So would only list sep of the two, and explain clearly what the default sep values means (you can even say it is equivalent to delim_whitespace=True).

keshavramaswamy · 2016-10-31T02:47:07Z

@jorisvandenbossche Thanks for your review. While that makes sense, as issue #13726 mentions, with both the arguments added, it'll be helpful for argument completion in interactive use. Thoughts?

jorisvandenbossche · 2016-10-31T20:39:12Z

with both the arguments added, it'll be helpful for argument completion in interactive use

The point I was trying to make is that in the current way they are specified, having both arguments is not useful: sep='\s+' is the default, and setting delim_whitespace to True is equivalent with this, so in practice you never need this.

Towards the users, I would maybe rather make delim_whitespace=True the default, as that is easier to understand what it means compared to sep='\s+'. But that is maybe a bit more complex to implement.
So to conclude, I would just document sep, and leave delim_whitespace out of the signature.

keshavramaswamy · 2016-11-01T13:35:27Z

@jorisvandenbossche ah, got it. Will remove delim_whitespace from the signature.

keshavramaswamy · 2016-11-02T04:00:03Z

@TomAugspurger Can this be merged?

gfyoung · 2016-11-02T18:36:04Z

pandas/io/tests/test_clipboard.py

@@ -71,6 +71,7 @@ def check_round_trip_frame(self, data_type, excel=None, sep=None):
    def test_round_trip_frame_sep(self):


Do you think you can add round-trip tests for slightly unusual delimiters instead of the standard white-space one? One is probably sufficient.

This actually just used read_csv under the hood, so I don't know to what extent this is needed? (apart from checking that the keyword is passed correctly)

True, but if the implementation changes, we would like to make sure such behaviour does not break. In any case, couldn't hurt, right? 😄

@gfyoung not sure what you exactly mean by 'round-trip tests', I added a test for pipe-delimiter though.

That's what the test method name is describing, which is why I put the comment under the name. What you did should be fine.

Thanks @gfyoung - got it.

jorisvandenbossche · 2016-11-02T19:56:14Z

pandas/io/clipboard.py

+    Parameters
+    ----------
+    sep : str, default '\s+'
+        A string or regex delimiter.


Can you add here an explanation of what \s+ means ?

Added that! @jorisvandenbossche

keshavramaswamy · 2016-11-06T23:49:51Z

Looks like the build fails due to this? pypa/pip#4059

jorisvandenbossche · 2016-11-07T20:53:03Z

@keshavramaswamy Thanks a lot! (I made a small adjustment before merging, and made the docstring a raw string, which is needed for sphinx to interpret the \ literally)

TomAugspurger reviewed Oct 30, 2016

View reviewed changes

jorisvandenbossche added Docs IO Data IO issues that don't fit into a more specific label labels Oct 30, 2016

jreback changed the title ~~DOC:added arguments to read_clipboard signature~~ DOC: added arguments to read_clipboard signature Oct 31, 2016

gfyoung reviewed Nov 2, 2016

View reviewed changes

jorisvandenbossche reviewed Nov 2, 2016

View reviewed changes

jorisvandenbossche added this to the 0.20.0 milestone Nov 2, 2016

added argument sep='\s+' to read_clipboard signature along with tests

c499722

make raw string for doc building

e8edce9

jorisvandenbossche changed the title ~~DOC: added arguments to read_clipboard signature~~ DOC: add sep argument to read_clipboard signature Nov 7, 2016

jorisvandenbossche merged commit 2e276fb into pandas-dev:master Nov 7, 2016

jreback mentioned this pull request Mar 4, 2017

pandas index does not give expected behaviour when raised to a power #15565

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: add sep argument to read_clipboard signature #14537

DOC: add sep argument to read_clipboard signature #14537

keshavramaswamy commented Oct 29, 2016 •

edited by jorisvandenbossche

Loading

codecov-io commented Oct 29, 2016 •

edited

Loading

TomAugspurger left a comment

TomAugspurger Oct 30, 2016 •

edited by jorisvandenbossche

Loading

TomAugspurger Oct 30, 2016

keshavramaswamy Oct 31, 2016

jorisvandenbossche commented Oct 30, 2016

keshavramaswamy commented Oct 31, 2016

jorisvandenbossche commented Oct 31, 2016

keshavramaswamy commented Nov 1, 2016

keshavramaswamy commented Nov 2, 2016

gfyoung Nov 2, 2016

jorisvandenbossche Nov 2, 2016

gfyoung Nov 2, 2016

keshavramaswamy Nov 4, 2016

gfyoung Nov 4, 2016 •

edited

Loading

keshavramaswamy Nov 7, 2016

jorisvandenbossche Nov 2, 2016

keshavramaswamy Nov 4, 2016

keshavramaswamy commented Nov 6, 2016

jorisvandenbossche commented Nov 7, 2016


		return read_table(StringIO(text), **kwargs)
		return read_table(StringIO(text), sep, delim_whitespace, **kwargs)

		@@ -71,6 +71,7 @@ def check_round_trip_frame(self, data_type, excel=None, sep=None):
		def test_round_trip_frame_sep(self):

DOC: add sep argument to read_clipboard signature #14537

DOC: add sep argument to read_clipboard signature #14537

Conversation

keshavramaswamy commented Oct 29, 2016 • edited by jorisvandenbossche Loading

codecov-io commented Oct 29, 2016 • edited Loading

Current coverage is 85.26% (diff: 100%)

TomAugspurger left a comment

Choose a reason for hiding this comment

TomAugspurger Oct 30, 2016 • edited by jorisvandenbossche Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorisvandenbossche commented Oct 30, 2016

keshavramaswamy commented Oct 31, 2016

jorisvandenbossche commented Oct 31, 2016

keshavramaswamy commented Nov 1, 2016

keshavramaswamy commented Nov 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Nov 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keshavramaswamy commented Nov 6, 2016

jorisvandenbossche commented Nov 7, 2016

keshavramaswamy commented Oct 29, 2016 •

edited by jorisvandenbossche

Loading

codecov-io commented Oct 29, 2016 •

edited

Loading

TomAugspurger Oct 30, 2016 •

edited by jorisvandenbossche

Loading

gfyoung Nov 4, 2016 •

edited

Loading