API/DOC: Deprecate and Advise against having `np.nan` in Categoricals

[TomAugspurger]

This came out of work on #10729

In the documentation, we mention that

There are two ways a np.nan can be represented in categorical data: either the value is not available (“missing value”) or np.nan is a valid category.

In the first case, NaN is not in .categories, and in the second case it is. I think we should only
recommend the first.

The option of NaNs in the categories makes the code in #10729 less pleasant that it would be otherwise. I don't think we should error if NaNs are included, just advise against it in the docs. Perhaps a deprecation, but I worry that I'm missing some obvious reason why NaNs were allowed in .categories.

@JanSchulz do you remember the initial reason for allowing either representation?

Some bad things that come out of NaN in .categories:

• Can't rely on a value of nan mapping to a code of -1:

In [2]: s = pd.Categorical(['a', 'b', 'a', np.nan], categories=['a', 'b', np.nan])

In [3]: s
Out[3]:
[a, b, a, NaN]
Categories (3, object): [a, b, NaN]

In [4]: s.categories
Out[4]: Index(['a', 'b', nan], dtype='object')

In [5]: s.codes
Out[5]: array([0, 1, 0, 2], dtype=int8)

• potentially have to upcast the index type or mix strings and floats (nan) in the .categories Index.
• extra code if you want to generically handle Categoricals that may or may not have NaN in categories.

[jreback]

hmm, we don't have an explicity prohibition in _validate_categories, but if you pass in your own categories (not as an Index) a null would be removed.

I think if you added a test and checked in validate_categories (maybe exempting fastpath=True) would be ok. (and I dont think anything would fail).

[jorisvandenbossche]

Unless someone comes up with a good use case of NaNs in the categories, I think we should just disallow it. This will make it a lot easier I think. Because even if we advise against it in the docs, if it is possible, people will do it and we will have to deal with the bug reports.

[TomAugspurger]

Ok, I've updated the title to recommend deprecation (unless there's a good reason otherwise). Later tonight I'll try out a check disallowing it and see what happens.

[jankatins]

The original reason, AFAIR, was that R handled that corner case (NA as factor and NA as missing -> two different NA in a vector). See here: https://stat.ethz.ch/R-manual/R-devel/library/base/html/factor.html

The codes of a factor may contain NA. For a numeric x, set exclude = NULL to make NA an extra level (prints as <NA>); by default, this is the last level.

If NA is a level, the way to set a code to be missing (as opposed to the code of the missing level) is to use is.na on the left-hand-side of an assignment (as in is.na(f)[i] <- TRUE; indexing inside is.na does not work). Under those circumstances missing values are currently printed as <NA>, i.e., identical to entries of level NA.

[TomAugspurger]

Thanks.

I think this is an area where with the pandas way of handling missing data (np.nan for everything) means it makes sense to differ from R. Unless the potential of adding integer NA with dynd means we would want to allow NA in the .categories. I still don't really see the benefit of that though.

[shoyer]

I agree, I don't think there is anything to be gained from copying this R behavior in pandas.

[jreback]

@TomAugspurger let's just change this. I don't even thing its worth deprecating.

[jorisvandenbossche]

@TomAugspurger any chance you would have some time for this to get it into 0.17?

[TomAugspurger]

I'll give it a shot today. I think FutureWarning though right, not a straight removal? People could be relying on this.