Incorporate review feedback

Author: h-vetinari

doc/source/text.rst

@@ -257,6 +257,7 @@ The parameter ``others`` can also be two-dimensional. In this case, the number o
 .. ipython:: python
 
     d = pd.concat([t, s], axis=1)
+    s
     d
     s.str.cat(d, na_rep='-')
     
@@ -271,6 +272,8 @@ the ``join``-keyword, which controls the manner of alignment.
 .. ipython:: python
 
     u = pd.Series(['b', 'd', 'a', 'c'], index=[1, 3, 0, 2])
+    s
+    u
     s.str.cat(u)
     s.str.cat(u, join='left')
 
@@ -285,6 +288,8 @@ In particular, alignment also means that the different lengths do not need to co
 .. ipython:: python
 
     v = pd.Series(['z', 'a', 'b', 'd', 'e'], index=[-1, 0, 1, 3, 4])
+    s
+    v
     s.str.cat(v, join='left', na_rep='-')
     s.str.cat(v, join='outer', na_rep='-')
 
@@ -293,13 +298,14 @@ The same alignment can be used when ``others`` is a ``DataFrame``:
 .. ipython:: python
     
     f = d.loc[[3, 2, 1, 0], :]
+    s
     f
     s.str.cat(f, join='left', na_rep='-')
 
 Concatenating a Series and many objects into a Series
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-All list-likes (as well as ``DataFrame`` and two-dimensional ``ndarray``) can be arbitrarily combined in a list-like container:
+All list-likes (including iterators,  ``dict``-views, etc.) can be arbitrarily combined in a list-like container:
 
 .. ipython:: python
 
@@ -317,15 +323,10 @@ the union of these indexes will be used as the basis for the final concatenation
 
 .. ipython:: python
 
+    u.loc[[3]]
+    v.loc[[-1, 0]]
     s.str.cat([u.loc[[3]], v.loc[[-1, 0]]], join='right', na_rep='-')
 
-Finally, the surrounding container can also be an :obj:`Iterable` other than a ``list`` (e.g. an iterator, or a ``dict``-view, etc.):
-
-.. ipython:: python
-    
-    from collections import OrderedDict
-    s.str.cat(d.to_dict('series', into=OrderedDict).values(), na_rep='-')
-
 Indexing with ``.str``
 ----------------------
 

doc/source/whatsnew/v0.23.0.txt

@@ -326,17 +326,6 @@ to ``'left'`` in a future version of pandas.
 
 In particular, ``others`` does not need to be of the same length as the calling ``Series`` (if both have an index and ``join is not None``).
 For more examples, see :ref:`here <text.concatenate>`.
-
-Additionally, ``str.cat`` now allows ``others`` to be a ``DataFrame`` or two-dimensional ``np.ndarray``.
-
-.. ipython:: python
-    
-    u = pd.Series(['b', 'd', 'a', 'c'], index=[1, 3, 0, 2])
-    d = pd.concat([s, u], axis=1)
-    t.str.cat(d.values)
-    s.str.cat(d, join='left', na_rep='-')
-
-Furthermore, any combination of "concatenateable" arguments can be passed in a list-like container (e.g. an iterator).
 
 For categorical data, it is now possible to call :meth:`Series.str.cat` for ``CategoricalIndex`` as well (previously raised a ``ValueError``).
 Finally, if ``others is not None``, the resulting ``Series``/``Index`` will now remain categorical if the calling

pandas/core/strings.py

@@ -1800,6 +1800,7 @@ class StringMethods(NoNewAttributesMixin):
     def __init__(self, data):
         self._validate(data)
         self._is_categorical = is_categorical_dtype(data)
+
         # .values.categories works for both Series/Index
         self._data = data.values.categories if self._is_categorical else data
         # save orig to blow up categoricals to the right type
@@ -1934,7 +1935,7 @@ def cons_row(x):
                 cons = self._orig._constructor
                 return cons(result, name=name, index=index)
 
-    def _str_cat_los(self, input, ignore_index=False):
+    def _get_series_list(self, others, ignore_index=False):
         """
         Auxiliary function for :meth:`str.cat`. Turn potentially mixed input
         into list of Series (elements without an index must match the length of
@@ -1943,7 +1944,7 @@ def _str_cat_los(self, input, ignore_index=False):
         Parameters
         ----------
         input : Series, DataFrame, np.ndarrary, list-like or list-like of those
-        ignore_index : Boolean
+        ignore_index : boolean, default False
             Determines whether to forcefully align with index of the caller
 
         Returns
@@ -1955,46 +1956,44 @@ def _str_cat_los(self, input, ignore_index=False):
         # once str.cat defaults to alignment, this function can be simplified;
         # will not need `ignore_index` and the second boolean output anymore
 
-        from pandas.core.index import Index
-        from pandas.core.series import Series
-        from pandas.core.frame import DataFrame
+        from pandas import Index, Series, DataFrame
 
         # self._orig is either Series or Index
         idx = self._orig if isinstance(self._orig, Index) else self._orig.index
 
-        if isinstance(input, Series):
-            los = [Series(input.values, index=idx) if ignore_index else input]
+        if isinstance(others, Series):
+            los = [Series(others.values, index=idx)
+                   if ignore_index else others]
             return (los, True)
-        elif isinstance(input, Index):
-            los = [Series(input.values,
-                          index=(idx if ignore_index else input))]
+        elif isinstance(others, Index):
+            los = [Series(others.values,
+                          index=(idx if ignore_index else others))]
             return (los, True)
-        elif isinstance(input, DataFrame):
+        elif isinstance(others, DataFrame):
             if ignore_index:
                 # without copy, this could change (the corresponding list
                 # element of) "others" that was passed to str.cat
-                input = input.copy()
-                input.index = idx
-            return ([input[x] for x in input], True)
-        elif isinstance(input, np.ndarray) and input.ndim == 2:
-            input = DataFrame(input, index=idx)
-            return ([input[x] for x in input], False)
-        elif is_list_like(input):
-            input = list(input)  # ensure iterators do not get read twice, etc.
-            if all(is_list_like(x) for x in input):
+                others = others.copy()
+                others.index = idx
+            return ([others[x] for x in others], True)
+        elif isinstance(others, np.ndarray) and others.ndim == 2:
+            others = DataFrame(others, index=idx)
+            return ([others[x] for x in others], False)
+        elif is_list_like(others):
+            others = list(others)  # ensure iterators do not get read twice etc
+            if all(is_list_like(x) for x in others):
                 los = []
                 fuwa = False
-                while input:
-                    tmp = self._str_cat_los(input.pop(0),
-                                            ignore_index=ignore_index)
+                while others:
+                    tmp = self._get_series_list(others.pop(0),
+                                                ignore_index=ignore_index)
                     los = los + tmp[0]
                     fuwa = fuwa or tmp[1]
                 return (los, fuwa)
             else:
-                return ([Series(input, index=idx)], False)
-        else:
-            raise ValueError('input must be Series, Index, DataFrame, '
-                             'np.ndarrary or list-like')
+                return ([Series(others, index=idx)], False)
+        raise ValueError('others must be Series, Index, DataFrame, '
+                         'np.ndarrary or list-like')
 
     def cat(self, others=None, sep=None, na_rep=None, join=None):
         """
@@ -2136,11 +2135,9 @@ def cat(self, others=None, sep=None, na_rep=None, join=None):
 
         For more examples, see :ref:`here <text.concatenate>`.
         """
-        from pandas.core.index import Index
-        from pandas.core.series import Series
-        from pandas.core.reshape.concat import concat
+        from pandas import Index, Series, concat
 
-        if isinstance(others, str):
+        if isinstance(others, compat.string_types):
             raise ValueError("Did you mean to supply a `sep` keyword?")
 
         if isinstance(self._orig, Index):
@@ -2156,8 +2153,8 @@ def cat(self, others=None, sep=None, na_rep=None, join=None):
 
         try:
             # turn anything in "others" into lists of Series
-            others, fuwa = self._str_cat_los(others,
-                                             ignore_index=(join is None))
+            tmp = self._get_series_list(others, ignore_index=(join is None))
+            others, fut_warn = tmp
         except ValueError:
             if join is None:
                 # legacy warning
@@ -2168,7 +2165,7 @@ def cat(self, others=None, sep=None, na_rep=None, join=None):
                                  'must all be of the same length as the '
                                  'calling Series/Index.')
 
-        if join is None and fuwa:
+        if join is None and fut_warn:
             warnings.warn("A future version of pandas will perform index "
                           "alignment when `others` is a Series/Index/"
                           "DataFrame (or a list-like containing one). To "

pandas/tests/test_strings.py

@@ -132,12 +132,12 @@ def test_cat(self):
         exp = np.array(['aa', NA, 'bb', 'bd', 'cfoo', NA], dtype=np.object_)
         tm.assert_almost_equal(result, exp)
 
-    @pytest.mark.parametrize('ser_or_ind', ['series', 'index'])
-    def test_str_cat(self, ser_or_ind):
+    @pytest.mark.parametrize('series_or_index', ['series', 'index'])
+    def test_str_cat(self, series_or_index):
         # test_cat above tests "str_cat" from ndarray to ndarray;
         # here testing "str.cat" from Series/Index to Series/Index/ndarray/list
         s = Index(['a', 'a', 'b', 'b', 'c', np.nan])
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             s = Series(s)
         t = Index(['a', np.nan, 'b', 'd', 'foo', np.nan])
 
@@ -156,7 +156,7 @@ def test_str_cat(self, ser_or_ind):
 
         # Series/Index with Index
         exp = Index(['aa', 'a-', 'bb', 'bd', 'cfoo', '--'])
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             exp = Series(exp)
         with tm.assert_produces_warning(expected_warning=FutureWarning):
             # FutureWarning to switch to alignment by default
@@ -187,19 +187,19 @@ def test_str_cat(self, ser_or_ind):
         with tm.assert_raises_regex(ValueError, rgx):
             s.str.cat(list(z))
 
-    @pytest.mark.parametrize('ser_or_ind', ['series', 'index'])
-    def test_str_cat_raises_intuitive_error(self, ser_or_ind):
+    @pytest.mark.parametrize('series_or_index', ['series', 'index'])
+    def test_str_cat_raises_intuitive_error(self, series_or_index):
         # https://github.com/pandas-dev/pandas/issues/11334
         s = Index(['a', 'b', 'c', 'd'])
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             s = Series(s)
         message = "Did you mean to supply a `sep` keyword?"
         with tm.assert_raises_regex(ValueError, message):
             s.str.cat('|')
         with tm.assert_raises_regex(ValueError, message):
             s.str.cat('    ')
 
-    @pytest.mark.parametrize('ser_or_ind, dtype_caller, dtype_target', [
+    @pytest.mark.parametrize('series_or_index, dtype_caller, dtype_target', [
         ('series', 'object', 'object'),
         ('series', 'object', 'category'),
         ('series', 'category', 'object'),
@@ -209,14 +209,15 @@ def test_str_cat_raises_intuitive_error(self, ser_or_ind):
         ('index', 'category', 'object'),
         ('index', 'category', 'category')
     ])
-    def test_str_cat_categorical(self, ser_or_ind, dtype_caller, dtype_target):
+    def test_str_cat_categorical(self, series_or_index,
+                                 dtype_caller, dtype_target):
         s = Index(['a', 'a', 'b', 'a'], dtype=dtype_caller)
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             s = Series(s)
         t = Index(['b', 'a', 'b', 'c'], dtype=dtype_target)
 
         exp = Index(['ab', 'aa', 'bb', 'ac'], dtype=dtype_caller)
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             exp = Series(exp)
         # Series/Index with Index
         with tm.assert_produces_warning(expected_warning=FutureWarning):
@@ -228,16 +229,16 @@ def test_str_cat_categorical(self, ser_or_ind, dtype_caller, dtype_target):
             # FutureWarning to switch to alignment by default
             assert_series_or_index_equal(s.str.cat(Series(t)), exp)
 
-    @pytest.mark.parametrize('ser_or_ind', ['series', 'index'])
-    def test_str_cat_mixed_inputs(self, ser_or_ind):
+    @pytest.mark.parametrize('series_or_index', ['series', 'index'])
+    def test_str_cat_mixed_inputs(self, series_or_index):
         s = Index(['a', 'b', 'c', 'd'])
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             s = Series(s)
         t = Series(['A', 'B', 'C', 'D'])
         d = concat([t, Series(s)], axis=1)
 
         exp = Index(['aAa', 'bBb', 'cCc', 'dDd'])
-        if ser_or_ind == 'series':
+        if series_or_index == 'series':
             exp = Series(exp)
         # Series/Index with DataFrame
         with tm.assert_produces_warning(expected_warning=FutureWarning):
@@ -288,18 +289,18 @@ def test_str_cat_mixed_inputs(self, ser_or_ind):
         with tm.assert_raises_regex(ValueError, rgx):
             s.str.cat([z, list(s)])
 
-    @pytest.mark.parametrize('ser_or_ind, join', [
+    @pytest.mark.parametrize('series_or_index, join', [
         ('series', 'left'), ('series', 'outer'),
         ('series', 'inner'), ('series', 'right'),
         ('index', 'left'), ('index', 'outer'),
         ('index', 'inner'), ('index', 'right')
     ])
-    def test_str_cat_align_indexed(self, ser_or_ind, join):
+    def test_str_cat_align_indexed(self, series_or_index, join):
         # https://github.com/pandas-dev/pandas/issues/18657
         s = Series(['a', 'b', 'c', 'd'], index=['a', 'b', 'c', 'd'])
         t = Series(['D', 'A', 'E', 'B'], index=['d', 'a', 'e', 'b'])
         sa, ta = s.align(t, join=join)
-        if ser_or_ind == 'index':
+        if series_or_index == 'index':
             s = Index(s)
             sa = Index(sa)