Deprecate ordered=None for CategoricalDtype

[TomAugspurger]

xref #26327 (comment).

We currently allow CategoricalDtype.ordered to be None. It should just be a bool (default False).

In [8]: CategoricalDtype(categories=None, ordered=None).ordered
# None

categories can be None, which means "infer later on". But (IIRC) we don't do that for ordered.

[jschendel]

But (IIRC) we don't do that for ordered.

There is some reliance on this for astype, where we allow users to update categories by specifying them in a CategoricalDtype without needing to worry about specifying the same ordered (i.e. leaving it as None).

Basically:

In [1]: import pandas as pd; pd.__version__
Out[1]: '0.25.0.dev0+520.g2ef50aea04'

In [2]: cdt1 = pd.api.types.CategoricalDtype(categories=list('cdab'), ordered=True)

In [3]: s = pd.Series(list('abcdaba'), dtype=cdt1)

In [4]: cdt2 = pd.api.types.CategoricalDtype(categories=list('cedafb'))

In [5]: cdt2.ordered is None
Out[5]: True

In [6]: s = s.astype(cdt2)

In [7]: s.dtype.ordered
Out[7]: True

Having ordered default to False would change the above behavior. Not that big of a deal though, and probably better to force users to be explicit about this.

[jorisvandenbossche]

Having ordered default to False would change the above behavior.

That would indeed be a breaking change, so should we maybe see if we need to deprecate it first? (not sure it is worth it)

Personally, seeing that example, I would say that's even an extra reason to change it, because I find it very confusing that doing a astype(CategoricalDtype(..)) does not follow the default but keeps the original data ordered attribute.

[TomAugspurger]

Thanks @jschendel. I agree with

probably better to force users to be explicit about this.

How do we actually deprecate this behavior without breaking things? Changing the default in CategoricalDtype.__init__ to False is a breaking change...

[jorisvandenbossche]

How do we actually deprecate this behavior without breaking things?

Change the default to some other sentinel that we can catch? (_default = object()) And then only raise the warning when there would actually be a change in behaviour (i.e. when he dtype is used to convert data where the original ordered would not match with the new default)
Will not be the most clean code though ...

[jschendel]

Change the default to some other sentinel that we can catch? (_default = object())

Is this necessary? My understanding is that a different sentinel is used when we want to distinguish between when a user explicitly passes None versus the default None being passed. In this case I don't think it matters and we'd want to warn in either case. Or am I overlooking a different usecase?

[jorisvandenbossche]

Ah, yes, you're right :) Of course, there still might be the case where the user actually passes None (CategoricalDtype(categories=[...], ordered=None), which is kind of pointless, but it currently allowed ... So the question is if we should warn for that case as well, or just hard break that in the future.

[jschendel]

Seems easiest to warn in both cases, and probably more user friendly too. Opening a PR shortly.