Fix 'observed' kwarg not doing anything on SeriesGroupBy

doc/source/whatsnew/v0.25.0.rst

@@ -483,6 +483,7 @@ Groupby/Resample/Rolling
 - Bug in :func:`pandas.core.groupby.GroupBy.agg` when applying a aggregation function to timezone aware data (:issue:`23683`)
 - Bug in :func:`pandas.core.groupby.GroupBy.first` and :func:`pandas.core.groupby.GroupBy.last` where timezone information would be dropped (:issue:`21603`)
 - Bug in :func:`pandas.core.groupby.GroupBy.size` when grouping only NA values (:issue:`23050`)
+- Bug in :func:`Series.groupby` where ``observed`` kwarg was previously ignored (:issue:`24880`)
 - Bug in :func:`Series.groupby` where using ``groupby`` with a :class:`MultiIndex` Series with a list of labels equal to the length of the series caused incorrect grouping (:issue:`25704`)
 - Ensured that ordering of outputs in ``groupby`` aggregation functions is consistent across all versions of Python (:issue:`25692`)
 - Ensured that result group order is correct when grouping on an ordered ``Categorical`` and specifying ``observed=True`` (:issue:`25871`, :issue:`25167`)

pandas/core/groupby/generic.py

@@ -25,15 +25,14 @@
 from pandas.core.dtypes.missing import isna, notna
 
 import pandas.core.algorithms as algorithms
-from pandas.core.arrays import Categorical
 from pandas.core.base import DataError, SpecificationError
 import pandas.core.common as com
 from pandas.core.frame import DataFrame
 from pandas.core.generic import NDFrame, _shared_docs
 from pandas.core.groupby import base
 from pandas.core.groupby.groupby import (
     GroupBy, _apply_docs, _transform_template)
-from pandas.core.index import CategoricalIndex, Index, MultiIndex
+from pandas.core.index import Index, MultiIndex
 import pandas.core.indexes.base as ibase
 from pandas.core.internals import BlockManager, make_block
 from pandas.core.series import Series
@@ -834,9 +833,10 @@ def _wrap_output(self, output, index, names=None):
             return Series(output, index=index, name=name)
 
     def _wrap_aggregated_output(self, output, names=None):
-        return self._wrap_output(output=output,
-                                 index=self.grouper.result_index,
-                                 names=names)
+        result = self._wrap_output(output=output,
+                                   index=self.grouper.result_index,
+                                   names=names)
+        return self._reindex_output(result)._convert(datetime=True)
 
     def _wrap_transformed_output(self, output, names=None):
         return self._wrap_output(output=output,
@@ -856,23 +856,28 @@ def _get_index():
             return index
 
         if isinstance(values[0], dict):
-            # GH #823
+            # GH #823 #24880
             index = _get_index()
-            result = DataFrame(values, index=index).stack()
+            result = self._reindex_output(DataFrame(values, index=index))
+            # if self.observed is False,
+            # keep all-NaN rows created while re-indexing
+            result = result.stack(dropna=self.observed)
             result.name = self._selection_name
             return result
 
-        if isinstance(values[0], (Series, dict)):
+        if isinstance(values[0], Series):
             return self._concat_objects(keys, values,
                                         not_indexed_same=not_indexed_same)
         elif isinstance(values[0], DataFrame):
             # possible that Series -> DataFrame by applied function
             return self._concat_objects(keys, values,
                                         not_indexed_same=not_indexed_same)
         else:
-            # GH #6265
-            return Series(values, index=_get_index(),
-                          name=self._selection_name)
+            # GH #6265 #24880
+            result = Series(data=values,
+                            index=_get_index(),
+                            name=self._selection_name)
+            return self._reindex_output(result)
 
     def _aggregate_named(self, func, *args, **kwargs):
         result = OrderedDict()
@@ -1335,7 +1340,8 @@ def _gotitem(self, key, ndim, subset=None):
             if subset is None:
                 subset = self.obj[key]
             return SeriesGroupBy(subset, selection=key,
-                                 grouper=self.grouper)
+                                 grouper=self.grouper,
+                                 observed=self.observed)
 
         raise AssertionError("invalid ndim for _gotitem")
 
@@ -1407,69 +1413,6 @@ def _wrap_agged_blocks(self, items, blocks):
 
         return self._reindex_output(result)._convert(datetime=True)
 
-    def _reindex_output(self, result):
-        """
-        If we have categorical groupers, then we want to make sure that
-        we have a fully reindex-output to the levels. These may have not
-        participated in the groupings (e.g. may have all been
-        nan groups);
-
-        This can re-expand the output space
-        """
-
-        # we need to re-expand the output space to accomodate all values
-        # whether observed or not in the cartesian product of our groupes
-        groupings = self.grouper.groupings
-        if groupings is None:
-            return result
-        elif len(groupings) == 1:
-            return result
-
-        # if we only care about the observed values
-        # we are done
-        elif self.observed:
-            return result
-
-        # reindexing only applies to a Categorical grouper
-        elif not any(isinstance(ping.grouper, (Categorical, CategoricalIndex))
-                     for ping in groupings):
-            return result
-
-        levels_list = [ping.group_index for ping in groupings]
-        index, _ = MultiIndex.from_product(
-            levels_list, names=self.grouper.names).sortlevel()
-
-        if self.as_index:
-            d = {self.obj._get_axis_name(self.axis): index, 'copy': False}
-            return result.reindex(**d)
-
-        # GH 13204
-        # Here, the categorical in-axis groupers, which need to be fully
-        # expanded, are columns in `result`. An idea is to do:
-        # result = result.set_index(self.grouper.names)
-        #                .reindex(index).reset_index()
-        # but special care has to be taken because of possible not-in-axis
-        # groupers.
-        # So, we manually select and drop the in-axis grouper columns,
-        # reindex `result`, and then reset the in-axis grouper columns.
-
-        # Select in-axis groupers
-        in_axis_grps = ((i, ping.name) for (i, ping)
-                        in enumerate(groupings) if ping.in_axis)
-        g_nums, g_names = zip(*in_axis_grps)
-
-        result = result.drop(labels=list(g_names), axis=1)
-
-        # Set a temp index and reindex (possibly expanding)
-        result = result.set_index(self.grouper.result_index
-                                  ).reindex(index, copy=False)
-
-        # Reset in-axis grouper columns
-        # (using level numbers `g_nums` because level names may not be unique)
-        result = result.reset_index(level=g_nums)
-
-        return result.reset_index(drop=True)
-
     def _iterate_column_groupbys(self):
         for i, colname in enumerate(self._selected_obj.columns):
             yield colname, SeriesGroupBy(self._selected_obj.iloc[:, i],

pandas/core/groupby/groupby.py

@@ -36,13 +36,14 @@ class providing the base-class of operations.
 from pandas.api.types import (
     is_datetime64_dtype, is_integer_dtype, is_object_dtype)
 import pandas.core.algorithms as algorithms
+from pandas.core.arrays import Categorical
 from pandas.core.base import (
     DataError, GroupByError, PandasObject, SelectionMixin, SpecificationError)
 import pandas.core.common as com
 from pandas.core.frame import DataFrame
 from pandas.core.generic import NDFrame
 from pandas.core.groupby import base
-from pandas.core.index import Index, MultiIndex
+from pandas.core.index import CategoricalIndex, Index, MultiIndex
 from pandas.core.series import Series
 from pandas.core.sorting import get_group_index_sorter
 
@@ -2301,6 +2302,79 @@ def tail(self, n=5):
         mask = self._cumcount_array(ascending=False) < n
         return self._selected_obj[mask]
 
+    def _reindex_output(self, output):
+        """
+        If we have categorical groupers, then we might want to make sure that
+        we have a fully re-indexed output to the levels. This means expanding
+        the output space to accommodate all values in the cartesian product of
+        our groups, regardless of whether they were observed in the data or
+        not. This will expand the output space if there are missing groups.
+
+        The method returns early without modifying the input if the number of
+        groupings is less than 2, self.observed == True or none of the groupers
+        are categorical.
+
+        Parameters
+        ----------
+        output: Series or DataFrame
+            Object resulting from grouping and applying an operation.
+
+        Returns
+        -------
+        Series or DataFrame
+            Object (potentially) re-indexed to include all possible groups.
+        """
+        groupings = self.grouper.groupings
+        if groupings is None:
+            return output
+        elif len(groupings) == 1:
+            return output
+
+        # if we only care about the observed values
+        # we are done
+        elif self.observed:
+            return output
+
+        # reindexing only applies to a Categorical grouper
+        elif not any(isinstance(ping.grouper, (Categorical, CategoricalIndex))
+                     for ping in groupings):
+            return output
+
+        levels_list = [ping.group_index for ping in groupings]
+        index, _ = MultiIndex.from_product(
+            levels_list, names=self.grouper.names).sortlevel()
+
+        if self.as_index:
+            d = {self.obj._get_axis_name(self.axis): index, 'copy': False}
+            return output.reindex(**d)
+
+        # GH 13204
+        # Here, the categorical in-axis groupers, which need to be fully
+        # expanded, are columns in `output`. An idea is to do:
+        # output = output.set_index(self.grouper.names)
+        #                .reindex(index).reset_index()
+        # but special care has to be taken because of possible not-in-axis
+        # groupers.
+        # So, we manually select and drop the in-axis grouper columns,
+        # reindex `output`, and then reset the in-axis grouper columns.
+
+        # Select in-axis groupers
+        in_axis_grps = ((i, ping.name) for (i, ping)
+                        in enumerate(groupings) if ping.in_axis)
+        g_nums, g_names = zip(*in_axis_grps)
+
+        output = output.drop(labels=list(g_names), axis=1)
+
+        # Set a temp index and reindex (possibly expanding)
+        output = output.set_index(self.grouper.result_index
+                                  ).reindex(index, copy=False)
+
+        # Reset in-axis grouper columns
+        # (using level numbers `g_nums` because level names may not be unique)
+        output = output.reset_index(level=g_nums)
+
+        return output.reset_index(drop=True)
+
 
 GroupBy._add_numeric_operations()
 

pandas/tests/groupby/test_categorical.py

@@ -1,3 +1,4 @@
+from collections import OrderedDict
 from datetime import datetime
 
 import numpy as np
@@ -963,3 +964,81 @@ def test_shift(fill_value):
                               categories=['a', 'b', 'c', 'd'], ordered=False)
     res = ct.shift(1, fill_value=fill_value)
     assert_equal(res, expected)
+
+
+class TestSeriesGroupByObservedKwarg:
+    # GH 24880
+
+    @pytest.fixture(autouse=True)
+    def setup_method(self, df):
+        self.df = df.copy()[:4]  # leave out some groups
+        self.df['A'] = self.df['A'].astype('category')
+        self.df['B'] = self.df['B'].astype('category')
+        self.df['C'] = pd.Series([1, 2, 3, 4])
+
+    @pytest.mark.parametrize('operation, index', [
+        ('agg', MultiIndex.from_frame(
+            pd.DataFrame({'A': ['foo', 'foo', 'bar', 'bar'],
+                          'B': ['one', 'two', 'one', 'three']
+                          }, dtype='category'))),
+        ('apply', MultiIndex.from_frame(
+            pd.DataFrame({'A': ['foo', 'foo', 'bar', 'bar'],
+                          'B': ['one', 'two', 'one', 'three']
+                          })))])
+    def test_true(self, operation, index):
+        expected = pd.Series(data=[1, 3, 2, 4], index=index, name='C')
+        grouped = self.df.groupby(['A', 'B'], observed=True)['C']
+        result = getattr(grouped, operation)(sum)
+        assert_series_equal(result, expected)
+
+    @pytest.mark.parametrize('operation', ['agg', 'apply'])
+    @pytest.mark.parametrize('observed', [False, None])
+    def test_false_or_none(self, observed, operation):
+        index, _ = MultiIndex.from_product(
+            [CategoricalIndex(['bar', 'foo'],
+                              categories=['bar', 'foo'],
+                              ordered=False),
+             CategoricalIndex(['one', 'three', 'two'],
+                              categories=['one', 'three', 'two'],
+                              ordered=False),
+             ],
+            names=['A', 'B']).sortlevel()
+
+        expected = pd.Series(data=[2, 4, np.nan, 1, np.nan, 3],
+                             index=index, name='C')
+        grouped = self.df.groupby(['A', 'B'], observed=observed)['C']
+        result = getattr(grouped, operation)(sum)
+        assert_series_equal(result, expected)
+
+    @pytest.mark.parametrize("observed, index, data", [
+        (True, MultiIndex.from_tuples(
+            [('foo', 'one', 'min'), ('foo', 'one', 'max'),
+             ('foo', 'two', 'min'), ('foo', 'two', 'max'),
+             ('bar', 'one', 'min'), ('bar', 'one', 'max'),
+             ('bar', 'three', 'min'), ('bar', 'three', 'max')],
+            names=['A', 'B', None]), [1, 1, 3, 3, 2, 2, 4, 4]),
+        (False, MultiIndex.from_product(
+            [CategoricalIndex(['bar', 'foo'],
+                              categories=['bar', 'foo'],
+                              ordered=False),
+             CategoricalIndex(['one', 'three', 'two'],
+                              categories=['one', 'three', 'two'],
+                              ordered=False),
+             Index(['min', 'max'])],
+            names=['A', 'B', None]),
+         [2, 2, 4, 4, np.nan, np.nan, 1, 1, np.nan, np.nan, 3, 3]),
+        (None, MultiIndex.from_product(
+            [CategoricalIndex(['bar', 'foo'],
+                              categories=['bar', 'foo'],
+                              ordered=False),
+             CategoricalIndex(['one', 'three', 'two'],
+                              categories=['one', 'three', 'two'],
+                              ordered=False),
+             Index(['min', 'max'])],
+            names=['A', 'B', None]),
+         [2, 2, 4, 4, np.nan, np.nan, 1, 1, np.nan, np.nan, 3, 3])])
+    def test_apply_dict(self, observed, index, data):
+        expected = pd.Series(data=data, index=index, name='C')
+        result = self.df.groupby(['A', 'B'], observed=observed)['C'].apply(
+            lambda x: OrderedDict([('min', x.min()), ('max', x.max())]))
+        assert_series_equal(result, expected)