[DEPR]: Deprecate setting nans in categories

[TomAugspurger]

WIP still

Closes #10748

I have to run for now, but will pick this up later today.
I think I'm missing a few in the tests, since the warning is showing up in a bunch of places (there's a way to convert those to errors for testing right?)

I had to refactor a couple function that were setting ._categories directly instead of using Categorical._set_categories. I kept that in a separate commit. Could do a bit more refactoring with the validate_categories stuff, but that can be separate.

And I need to figure out the proper stacklevel for this warning. I think I used 3 for now.

[jreback]

need to change the setting of categories in the fastpath section (near top of __init__). and add a fastpath to avoid this check.

[TomAugspurger]

fastpath here goes thru set_categories which calls ._set_categories. It still needs to go through the validate part so I'm not really sure what it's fastpathing.

All the warnings are caught in the test_categorical. There were quite a few.

[jreback]

that's my point - if I am fast pathing I don't need any validation at all (so I want to call _set_categories) to assign them but not validate

rather than

self._categories = ...

[TomAugspurger]

I think fastpath is being misused here. Some tests failed when I changed the constructor to do things properly (fastpath=True -> set categories directly, no validation). For one example (from the tests):

factor = Categorical([0,1,2,0,1,2]*100, ['a', 'b', 'c'], name='cat', fastpath=True)

breaks since it's assumed elsewhere that categories is always an Index. Followup in a separate issue?

[jreback]

fast path is purely internal - the point is you already have things setup correctly and simply easy to fall the constructor

your example is not valid

[TomAugspurger]

My example came from the test suite :)

There's about 8 tests that fail / error if I change the constructor to skip category validation when fastpath is true. Not sure if I'll be able to get to those today.

[jreback]

hmm this extra validation is a problem
fast path creation of categorically is already slow

[TomAugspurger]

@jreback cherry picked your PR, thanks for that. I can squash down to one commit if you want.

I'll try to do a perf test today.

[jreback]

squash up to you. yeh just do a quick perf test. merge when green.

[jorisvandenbossche]

I think this should be on the previous line?

[TomAugspurger]

@jorisvandenbossche Addressed your 3 points. I also

• Changed the level heading for Differences to R's factors to move it out one (it was under the Missing Data section)
• Added a note to the Differences section say that R does allow missing levels / categories, while pandas doesn't.

running asv now. The only categorical test is in concat (not sure if this touches the fastpath constructor). Might add another test.

[TomAugspurger]

@jreback

In [14]: %timeit Categorical(codes, cat_idx, fastpath=True) 100 loops, best of 3: 4.01 ms per loop In [15]: %timeit Categorical(values, cat_idx) 1 loops, best of 3: 279 ms per loop

👍 I've got an asv benchmark here. Just wrote and pushed an asv test for the Categorical constructor. Running it before and after your commit now. I'm guessing it should be similar to those numbers though since fastpath wasn't really skipping much before.

[jreback]

@TomAugspurger awesome

yeh in dask Categoricals are created a lot and just want to fastpath them (e.g. with from_codes), but the is_unique check is not necessary (but is currently done) on a fair amount. So this is good.

[TomAugspurger]

asv doesn't show much of a difference (assuming I ran this correctly).

asv run 30f672c..8d87f3b --bench=categorical_constructor · Fetching recent changes · Creating environments · Discovering benchmarks ·· Uninstalling from py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt. ·· Building for py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt............................... ·· Installing into py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt.. · Running 4 total benchmarks (2 commits * 1 environments * 2 benchmarks) [ 0.00%] · For pandas commit hash 8d87f3be: [ 0.00%] ·· Building for py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt................................... [ 0.00%] ·· Benchmarking py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt [ 25.00%] ··· Running categoricals.categorical_constructor.time_fastpath 5.84ms [ 50.00%] ··· Running categoricals.categorical_constructor.time_regular_constructor 250.19ms [ 50.00%] · For pandas commit hash e757e8a8: [ 50.00%] ·· Building for py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt...................................... [ 50.00%] ·· Benchmarking py3.4-Cython-matplotlib-numexpr-numpy-openpyxl-scipy-sqlalchemy-tables-xlrd-xlwt [ 75.00%] ··· Running categoricals.categorical_constructor.time_fastpath 4.83ms [100.00%] ··· Running categoricals.categorical_constructor.time_regular_constructor 246.23ms

Those two hashes are

8d87f3b move NaN deprecation warning to _validate_categories, cleanup a bit e757e8a DEPR: No NaNs in categories 

[jreback]

ok, merge away then

[TomAugspurger]

Merged, thanks.