API: remove compat keyword in Categorical constructor

Before the new Categorical work, the default two argument constructor
was expecting "codes and levels" but this was changed to
"values and levels" and a 'compat' kwarg was added, which could
be used to switch to the old style constructor useage.

It was decided that we switch to the new style constructor useage as
the new default (compat=False) and implement a 'from_codes(...)'
constructor.

As the compat codepath is now never triggered and code should be
changed to use the new 'from_codes()' constructor, remove the old
codepath.

Add some warnings if we are pretty sure that the old style
constructor is meant, but unfortunately we don't catch all cases.

Author: jankatins

doc/source/v0.15.0.txt

@@ -40,6 +40,13 @@ users upgrade to this version.
    but instead subclass ``PandasObject``, similarly to the rest of the pandas objects. This change allows very easy sub-classing and creation of new index types. This should be
    a transparent change with only very limited API implications (See the :ref:`Internal Refactoring <whatsnew_0150.refactoring>`)
 
+.. warning::
+
+   The refactorings in :class:`~pandas.Categorical` changed the two argument constructor from
+   "codes/labels and levels" to "values and levels". This can lead to subtle bugs. If you use
+   :class:`~pandas.Categorical` directly, please audit your code before updating to this pandas
+   version and change it to use the :meth:`~pandas.Categorical.from_codes` constructor.
+
 .. _whatsnew_0150.api:
 
 API changes
@@ -575,9 +582,10 @@ For full docs, see the :ref:`Categorical introduction <categorical>` and the
 - ``pandas.core.group_agg`` and ``pandas.core.factor_agg`` were removed. As an alternative, construct
   a dataframe and use ``df.groupby(<group>).agg(<func>)``.
 
-- Supplying "codes/labels and levels" to the :class:`~pandas.Categorical` constructor is deprecated and does
-  not work without supplying ``compat=True``. The default mode now uses "values and levels".
-  Please change your code to use the :meth:`~pandas.Categorical.from_codes` constructor.
+- Supplying "codes/labels and levels" to the :class:`~pandas.Categorical` constructor is not
+  supported anymore. Supplying two arguments to the constructor is now interpreted as
+  "values and levels". Please change your code to use the :meth:`~pandas.Categorical.from_codes`
+  constructor.
 
 - The ``Categorical.labels`` attribute was renamed to ``Categorical.codes`` and is read
   only. If you want to manipulate codes, please use one of the

pandas/core/categorical.py

@@ -127,8 +127,6 @@ class Categorical(PandasObject):
     name : str, optional
         Name for the Categorical variable. If name is None, will attempt
         to infer from values.
-    compat : boolean, default=False
-        Whether to treat values as codes to the levels (old API, deprecated)
 
     Attributes
     ----------
@@ -197,7 +195,7 @@ class Categorical(PandasObject):
     # For comparisons, so that numpy uses our implementation if the compare ops, which raise
     __array_priority__ = 1000
 
-    def __init__(self, values, levels=None, ordered=None, name=None, fastpath=False, compat=False):
+    def __init__(self, values, levels=None, ordered=None, name=None, fastpath=False):
 
         if fastpath:
             # fast path
@@ -257,32 +255,29 @@ def __init__(self, values, levels=None, ordered=None, name=None, fastpath=False,
                     raise TypeError("'values' is not ordered, please explicitly specify the level "
                                     "order by passing in a level argument.")
         else:
-            # there are two ways if levels are present
-            # the old one, where each value is a int pointer to the levels array
-            # the new one, where each value is also in the level array (or np.nan)
+            # there were two ways if levels are present
+            # - the old one, where each value is a int pointer to the levels array -> not anymore
+            #   possible, but code outside of pandas could call us like that, so make some checks
+            # - the new one, where each value is also in the level array (or np.nan)
 
             # make sure that we always have the same type here, no matter what we get passed in
             levels = self._validate_levels(levels)
 
-            # There can be two ways: the old which passed in codes and levels directly
-            # and values have to be inferred and the new  one, which passes in values and levels
-            # and _codes have to be inferred.
-
-            # min and max can be higher and lower if not all levels are in the values
-            if compat and (com.is_integer_dtype(values) and
-                               (np.min(values) >= -1) and (np.max(values) < len(levels))):
-                warn("Using 'values' as codes is deprecated.\n"
-                     "'Categorical(... , compat=True)' is only there for historical reasons and "
-                     "should not be used in new code!\n"
-                     "See https://github.com/pydata/pandas/pull/7217", FutureWarning)
-                codes = values
-            else:
-                codes = _get_codes_for_values(values, levels)
+            codes = _get_codes_for_values(values, levels)
 
-                # if we got levels, we can assume that the order is intended
-                # if ordered is unspecified
-                if ordered is None:
-                    ordered = True
+            # TODO: check for old style usage. These warnings should be removes after 0.18/ in 2016
+            if com.is_integer_dtype(values) and not com.is_integer_dtype(levels):
+                warn("Values and Levels have different dtypes. Did you mean to use\n"
+                     "'Categorical.from_codes(codes, levels)'?", RuntimeWarning)
+
+            if com.is_integer_dtype(values) and (codes == -1).all():
+                warn("None of the levels were found in values. Did you mean to use\n"
+                     "'Categorical.from_codes(codes, levels)'?", RuntimeWarning)
+
+            # if we got levels, we can assume that the order is intended
+            # if ordered is unspecified
+            if ordered is None:
+                ordered = True
 
         self.ordered = False if ordered is None else ordered
         self._codes = codes

pandas/tests/test_categorical.py

@@ -39,31 +39,8 @@ def test_constructor_unsortable(self):
         self.assertFalse(factor.ordered)
 
     def test_constructor(self):
-        # There are multiple ways to call a constructor
 
-        # old style: two arrays, one a pointer to the labels
-        # old style is now only available with compat=True
         exp_arr = np.array(["a", "b", "c", "a", "b", "c"])
-        with tm.assert_produces_warning(FutureWarning):
-            c_old = Categorical([0,1,2,0,1,2], levels=["a","b","c"], compat=True)
-        self.assert_numpy_array_equal(c_old.__array__(), exp_arr)
-        # the next one are from the old docs
-        with tm.assert_produces_warning(FutureWarning):
-           c_old2 = Categorical([0, 1, 2, 0, 1, 2], [1, 2, 3], compat=True)
-        self.assert_numpy_array_equal(c_old2.__array__(), np.array([1, 2, 3, 1, 2, 3]))
-        with tm.assert_produces_warning(FutureWarning):
-            c_old3 = Categorical([0,1,2,0,1,2], ['a', 'b', 'c'], compat=True)
-        self.assert_numpy_array_equal(c_old3.__array__(), np.array(['a', 'b', 'c', 'a', 'b', 'c']))
-
-        with tm.assert_produces_warning(FutureWarning):
-            cat = pd.Categorical([1,2], levels=[1,2,3], compat=True)
-        self.assert_numpy_array_equal(cat.__array__(), np.array([2,3]))
-
-        with tm.assert_produces_warning(None):
-            cat = pd.Categorical([1,2], levels=[1,2,3], compat=False)
-        self.assert_numpy_array_equal(cat.__array__(), np.array([1,2]))
-
-        # new style
         c1 = Categorical(exp_arr)
         self.assert_numpy_array_equal(c1.__array__(), exp_arr)
         c2 = Categorical(exp_arr, levels=["a","b","c"])
@@ -174,6 +151,21 @@ def f():
         self.assertTrue(len(cat.codes) == 1)
         self.assertTrue(cat.codes[0] == 0)
 
+        # Catch old style constructor useage: two arrays, codes + levels
+        # We can only catch two cases:
+        #  - when the first is an integer dtype and the second is not
+        #  - when the resulting codes are all -1/NaN
+        with tm.assert_produces_warning(RuntimeWarning):
+            c_old = Categorical([0,1,2,0,1,2], levels=["a","b","c"])
+
+        with tm.assert_produces_warning(RuntimeWarning):
+            c_old = Categorical([0,1,2,0,1,2], levels=[3,4,5])
+
+        # the next one are from the old docs, but unfortunately these don't trigger :-(
+        with tm.assert_produces_warning(None):
+            c_old2 = Categorical([0, 1, 2, 0, 1, 2], [1, 2, 3])
+            cat = Categorical([1,2], levels=[1,2,3])
+
     def test_constructor_with_generator(self):
         # This was raising an Error in isnull(single_val).any() because isnull returned a scalar
         # for a generator