DOC: update str.cat example #23723

h-vetinari · 2018-11-15T18:57:30Z

There have been a couple of changes recently due to the whole docstring validation effort (e.g. #22838), which got rid of some warnings emitted by the sample code.

As it stands, the code samples don't really reflect the running text anymore, and with the changes from #22264, it makes sense to update the running text as well.

h-vetinari · 2018-11-15T20:26:53Z

Just realised while double-checking the code locally that there's still a FutureWarning, which did not get caught by any test after the pd.concat(sort=...)-transition, because it was being shadowed by the FutureWarning for join is None.

In other words, the doc build here will have a warning (=fail?), until #23725 is merged.

h-vetinari · 2018-11-15T20:39:49Z

Ok, the build didn't fail - it's just a flaky hypothesis failure.

jreback · 2018-11-17T23:12:50Z

doc/source/text.rst

-    u
-    s.str.cat([u.values,
-               u.index.astype(str).values], na_rep='-')
+    s2 = s.set_axis(['a', 'b', 'c', 'd'], inplace=False)


this is not obvious at all, just directly construct the Series will be much more clear

jreback · 2018-11-17T23:12:56Z

doc/source/text.rst

-               u.index.astype(str).values], na_rep='-')
+    s2 = s.set_axis(['a', 'b', 'c', 'd'], inplace=False)
+    s2
+    u2 = u.set_axis(['b', 'd', 'a', 'c'], inplace=False)


WillAyd · 2018-11-17T23:30:36Z

doc/source/text.rst


 .. ipython:: python

    v
-    s.str.cat([u, v], join='outer', na_rep='-')
+    s.str.cat([v, u, u.values], join='outer', na_rep='-')


Instead of accessing .values would also be preferable to use the container here, assuming you build one to instantiate a Series on next update

codecov · 2018-11-18T12:16:16Z

Codecov Report

Merging #23723 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master   #23723   +/-   ##
=======================================
  Coverage   92.38%   92.38%           
=======================================
  Files         166      166           
  Lines       52490    52490           
=======================================
  Hits        48493    48493           
  Misses       3997     3997

Flag	Coverage Δ
#multiple	`90.81% <ø> (ø)`	⬆️
#single	`43.05% <ø> (ø)`	⬆️

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 6249355...3f8d4fd. Read the comment docs.

WillAyd · 2018-11-18T16:21:01Z

doc/source/text.rst

-    u
-    s.str.cat([u.values,
-               u.index.astype(str).values], na_rep='-')
+    s_values = np.array(['a', 'b', 'c', 'd'], dtype=object)  # same as s.values


I'd remove the comments here. Rather harmless with the example provided but I don't know if that comment will hold universally with all types (thinking EAs in particular) so don't want to give users an impression of that without context

Hm, I though this was clearly referencing the actual series s (as I wanted to motivate the variable name), rather than make a general statement about the relationship between np.array and .values.

i agree, the comment is just confusing

jreback · 2018-11-18T18:13:21Z

doc/source/text.rst

@@ -306,21 +306,26 @@ The same alignment can be used when ``others`` is a ``DataFrame``:
 Concatenating a Series and many objects into a Series
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-All one-dimensional list-likes can be combined in a list-like container (including iterators, ``dict``-views, etc.):
+Several items can be combined a list-like container (including iterators, ``dict``-views, etc.), which may contain ``Series``, ``Index`` and ``np.ndarray``.
+Note that ``Index`` will align as well, so we change the indexes of ``s`` and ``u`` to strings for the purpose of this example:


not sure what this last statement about Index actually means. can you reword.

jreback · 2018-11-18T18:14:26Z

doc/source/text.rst

-    u
-    s.str.cat([u.values,
-               u.index.astype(str).values], na_rep='-')
+    s_values = np.array(['a', 'b', 'c', 'd'], dtype=object)  # same as s.values


i agree, the comment is just confusing

jreback · 2018-11-20T02:28:49Z

this example still is a bit complicated. can you simplify at all.

h-vetinari · 2018-11-20T17:04:29Z

I don't like the example too much either, especially initialising the ndarray directly. After all, what I want to do is change the index of s to be the same as its values (due to alignment, as I note in the running test). For this, it might make sense to wait for #22225, because then it would be very easy:

>>> s2 = s.set_index(s.values)
>>> s2
a    a
b    b
c    c
d    d
dtype: object
>>> u2 = u.set_index(u.values)
>>> u2
b    b
d    d
a    a
c    c
dtype: object
>>> idx = pd.Index(['d', 'c', 'b', 'a'])
>>> idx
Index(['d', 'c', 'b', 'a'], dtype='object')
>>> u_values = u.values
>>> u_values
array(['b', 'd', 'a', 'c'], dtype=object)
>>> s2.str.cat([u2, idx, u_values], join='left')
a    aaab
b    bbbd
c    ccca
d    dddc
dtype: object

jreback · 2018-12-09T19:14:03Z

can you merge master and will look

h-vetinari · 2018-12-10T05:28:19Z

@jreback
Could you have a look at #24046? #22225 is essentially ready and would make things much easier here.

I'll merge in master later.

jreback · 2018-12-23T23:40:13Z

doc/source/text.rst

-               u.index.astype(str).values], na_rep='-')
+    s_values = np.array(['a', 'b', 'c', 'd'], dtype=object)
+    s2 = pd.Series(s_values, index=s_values)
+    s2


add this in multiple blocks as its too much to complicated here

this whole example needs to be simpler. maybe just leave the index as integers to avoid confusion, IOW focus less on the join in str.cat and more on the list-lke things that are going on.

The thing this example tries to show is that it works for Series, ndarray, and Index, and Index can only be aligned with a non-integer index. This whole example would be so so much easier with #22225. I've opened an issue for the DF discussion a while ago, and asked you to comment #24046.

h-vetinari

I have very limited availability over the holidays. Will consider removing the Index portion completely, but asking you to please take a look at #22225 / #24046.

h-vetinari · 2018-12-24T13:05:28Z

doc/source/text.rst

-               u.index.astype(str).values], na_rep='-')
+    s_values = np.array(['a', 'b', 'c', 'd'], dtype=object)
+    s2 = pd.Series(s_values, index=s_values)
+    s2


The thing this example tries to show is that it works for Series, ndarray, and Index, and Index can only be aligned with a non-integer index. This whole example would be so so much easier with #22225. I've opened an issue for the DF discussion a while ago, and asked you to comment #24046.

h-vetinari · 2019-01-02T19:08:52Z

@jreback
Incorporated your feedback (removed Index from example pending progress on #22225, as the extra complexity to align an index without having Series.set_index bloated the example).

It's all green, except for #24564

h-vetinari · 2019-01-03T01:02:01Z

@WillAyd
Your review is outdated, would you care to comment (or at least remove the change request), please?

WillAyd · 2019-01-03T02:21:22Z

doc/source/text.rst

@@ -303,23 +303,23 @@ The same alignment can be used when ``others`` is a ``DataFrame``:
 Concatenating a Series and many objects into a Series
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-All one-dimensional list-likes can be combined in a list-like container (including iterators, ``dict``-views, etc.):
+Several items can be combined a list-like container (including iterators, ``dict``-views, etc.), which may contain ``Series``, ``Index``, ``PandasArray`` and ``np.ndarray``.


Think it was a typo to remove the word in here

fixed, thanks

WillAyd · 2019-01-03T02:22:48Z

doc/source/text.rst

@@ -303,23 +303,23 @@ The same alignment can be used when ``others`` is a ``DataFrame``:
 Concatenating a Series and many objects into a Series
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-All one-dimensional list-likes can be combined in a list-like container (including iterators, ``dict``-views, etc.):
+Several items can be combined a list-like container (including iterators, ``dict``-views, etc.), which may contain ``Series``, ``Index``, ``PandasArray`` and ``np.ndarray``.


Also some nits around verbage, but I think it would be easier to keep the Series, Index, etc... mentions closer to "Several items"; as is I had to read a few times to truly understand what was meant after the word which

Maybe even just `Several items (ex: Series, Index, ...)

Done. I'm making an explicit list though, as only those types are allowed within list-likes

WillAyd · 2019-01-03T02:26:28Z

doc/source/text.rst


-All elements must match in length to the calling ``Series`` (or ``Index``), except those having an index if ``join`` is not None:
+All elements without an index (e.g. ``PandasArray`` and ``np.ndarray``) within the passed list-like must match in length to the calling ``Series`` (or ``Index``),


Might have missed this but what's the reason for bringing up PandasArray? Not really something the end user would be using directly (at least in current form)

PandasArray is very user-facing:

>>> s = pd.Series(['a', 'b' ,'c', 'd']) >>> s.array <PandasArray> ['a', 'b', 'c', 'd'] Length: 4, dtype: object

and the current example was recently changed to use .array instead of .values. I think this should be documented clearly.

OK thanks. I have been somewhat on the sidelines for that conversation so I'll defer to @jreback specifically on this piece

You don't need to mention PandasArray here, its not very interesting, nor relevant. Just say array-likes. and remove u.array, the u.to_numpy() is the corrent idiom here.

@jreback
There is a big difference between u.array and u.to_numpy():

>>> s = pd.Series(['a', 'b', 'c', 'd']) >>> s.array <PandasArray> ['a', 'b', 'c', 'd'] Length: 4, dtype: object >>> s.to_numpy() array(['a', 'b', 'c', 'd'], dtype=object)

I'm guessing .array will eventually replace .values-usage (e.g. to get rid of the index for .str.cat), since it is by design better suited for pandas-internal dtypes, and so the distinction above is not just an irrelevant detail IMO.

I want to show here the explicitly allowed item types to pass into a list-like, which have to pass:

nxt = others.pop(0) [...] if not (isinstance(nxt, (Series, Index)) or (isinstance(nxt, np.ndarray) and nxt.ndim == 1)): raise ValueError(...) # currently just a DeprecationWarning

Long story short, I want to show a list-like containing an np.ndarray, a PandasArray (to reiterate, this example was already changed by @TomAugspurger to use .array instead of .values in #23623), and a Series. (Including Index would be nice-to-have, but too complicated absent #22225).

@h-vetinari .to_numpy() replaces .values; .array is user-accessible, but generally is not visible to by the user. its not necessary here and is just noise.

@jreback

.to_numpy() replaces .values;

As I did in the last few commits...

.array is user-accessible, but generally is not visible to by the user. its not necessary here and is just noise.

Will remove, but pinging @TomAugspurger since he added the current u.array in this example in #23623 (although likely "just" for replacing u.values?).

h-vetinari

Thanks for review; pushed new commit

h-vetinari · 2019-01-03T10:46:22Z

doc/source/text.rst

@@ -303,23 +303,23 @@ The same alignment can be used when ``others`` is a ``DataFrame``:
 Concatenating a Series and many objects into a Series
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-All one-dimensional list-likes can be combined in a list-like container (including iterators, ``dict``-views, etc.):
+Several items can be combined a list-like container (including iterators, ``dict``-views, etc.), which may contain ``Series``, ``Index``, ``PandasArray`` and ``np.ndarray``.


fixed, thanks

h-vetinari · 2019-01-03T10:47:01Z

doc/source/text.rst

@@ -303,23 +303,23 @@ The same alignment can be used when ``others`` is a ``DataFrame``:
 Concatenating a Series and many objects into a Series
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

-All one-dimensional list-likes can be combined in a list-like container (including iterators, ``dict``-views, etc.):
+Several items can be combined a list-like container (including iterators, ``dict``-views, etc.), which may contain ``Series``, ``Index``, ``PandasArray`` and ``np.ndarray``.


Done. I'm making an explicit list though, as only those types are allowed within list-likes

h-vetinari · 2019-01-03T10:50:10Z

doc/source/text.rst


-All elements must match in length to the calling ``Series`` (or ``Index``), except those having an index if ``join`` is not None:
+All elements without an index (e.g. ``PandasArray`` and ``np.ndarray``) within the passed list-like must match in length to the calling ``Series`` (or ``Index``),


PandasArray is very user-facing:

>>> s = pd.Series(['a', 'b' ,'c', 'd']) >>> s.array <PandasArray> ['a', 'b', 'c', 'd'] Length: 4, dtype: object

and the current example was recently changed to use .array instead of .values. I think this should be documented clearly.

WillAyd

LGTM otherwise

WillAyd · 2019-01-03T15:37:12Z

doc/source/text.rst


-All elements must match in length to the calling ``Series`` (or ``Index``), except those having an index if ``join`` is not None:
+All elements without an index (e.g. ``PandasArray`` and ``np.ndarray``) within the passed list-like must match in length to the calling ``Series`` (or ``Index``),


OK thanks. I have been somewhat on the sidelines for that conversation so I'll defer to @jreback specifically on this piece

h-vetinari

PTAL

h-vetinari · 2019-01-03T20:35:08Z

doc/source/text.rst


-All elements must match in length to the calling ``Series`` (or ``Index``), except those having an index if ``join`` is not None:
+All elements without an index (e.g. ``PandasArray`` and ``np.ndarray``) within the passed list-like must match in length to the calling ``Series`` (or ``Index``),


@jreback

.to_numpy() replaces .values;

As I did in the last few commits...

.array is user-accessible, but generally is not visible to by the user. its not necessary here and is just noise.

Will remove, but pinging @TomAugspurger since he added the current u.array in this example in #23623 (although likely "just" for replacing u.values?).

jreback · 2019-01-04T12:36:24Z

thanks @h-vetinari

DOC: update str.cat example

7982f69

h-vetinari mentioned this pull request Nov 15, 2018

BUG: concat warning bubbling up through str.cat #23725

Merged

gfyoung added Docs Strings String extension data type and string data labels Nov 16, 2018

jreback requested changes Nov 17, 2018

View reviewed changes

WillAyd requested changes Nov 17, 2018

View reviewed changes

h-vetinari added 2 commits November 18, 2018 13:06

Merge remote-tracking branch 'upstream/master' into str_cat_docs

2655e8b

Review (jreback & WillAyd)

75ef297

WillAyd reviewed Nov 18, 2018

View reviewed changes

jreback requested changes Nov 18, 2018

View reviewed changes

Review (WillAyd & jreback)

f744052

Merge remote-tracking branch 'upstream/master' into str_cat_docs

2180da6

jreback requested changes Dec 23, 2018

View reviewed changes

h-vetinari commented Dec 24, 2018

View reviewed changes

Merge and remove Index from example (review jreback)

ccf14a3

TomAugspurger mentioned this pull request Jan 2, 2019

BLD/CI: conda cannot resolve two azure environments #24564

Closed

WillAyd requested changes Jan 3, 2019

View reviewed changes

h-vetinari added 2 commits January 3, 2019 11:29

Merge remote-tracking branch 'upstream/master' into str_cat_docs

a381c95

Review (WillAyd)

fc1d2dd

h-vetinari commented Jan 3, 2019

View reviewed changes

WillAyd approved these changes Jan 3, 2019

View reviewed changes

h-vetinari added 2 commits January 3, 2019 19:29

Merge remote-tracking branch 'upstream/master' into str_cat_docs

5d59dd1

Review (jreback)

b75c415

h-vetinari commented Jan 3, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into str_cat_docs

3f8d4fd

jreback added this to the 0.24.0 milestone Jan 4, 2019

jreback approved these changes Jan 4, 2019

View reviewed changes

jreback merged commit 25c1c38 into pandas-dev:master Jan 4, 2019

h-vetinari deleted the str_cat_docs branch January 4, 2019 16:58

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: update str.cat example (pandas-dev#23723)

4b51e01

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

DOC: update str.cat example (pandas-dev#23723)

32eeb33

h-vetinari mentioned this pull request Jul 26, 2019

DEPR: execute deprecations for str.cat in v1.0 #27611

Merged

3 tasks


		All elements must match in length to the calling ``Series`` (or ``Index``), except those having an index if ``join`` is not None:
		All elements without an index (e.g. ``PandasArray`` and ``np.ndarray``) within the passed list-like must match in length to the calling ``Series`` (or ``Index``),

DOC: update str.cat example #23723

DOC: update str.cat example #23723

Conversation

h-vetinari commented Nov 15, 2018

h-vetinari commented Nov 15, 2018

h-vetinari commented Nov 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 18, 2018 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

jreback commented Nov 20, 2018

h-vetinari commented Nov 20, 2018

jreback commented Dec 9, 2018

h-vetinari commented Dec 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari commented Jan 2, 2019

h-vetinari commented Jan 3, 2019

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari Jan 3, 2019 • edited Loading

Choose a reason for hiding this comment

h-vetinari left a comment

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

WillAyd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-vetinari left a comment

Choose a reason for hiding this comment

h-vetinari Jan 3, 2019 • edited Loading

Choose a reason for hiding this comment

jreback commented Jan 4, 2019

codecov bot commented Nov 18, 2018 •

edited

Loading

h-vetinari Jan 3, 2019 •

edited

Loading

h-vetinari Jan 3, 2019 •

edited

Loading