ENH: Categorical.fillna allow Categorical/ndarray #32420
Conversation
|
I think the failing test case is consistent with accidentally using a code for a value? Perhaps test with a Categorical and ndarray containing something other than integers to see if that's the case. |
|
Looks like the problem is that ATM Categorical.fillna is altering in-place for Series |
| return comps._values.isin(values) | ||
|
|
||
| comps = com.values_from_object(comps) | ||
| return comps.isin(values) # type: ignore |
There was a problem hiding this comment.
bc mypy doesnt know enough to rule out ndarray (or even non-Categorical EA) here
There was a problem hiding this comment.
not sure i understand. shouldn't you just add
result = comps.isin(values)
assert isinstance(result, np.ndarray)
return result
prefer this to a type ignore
There was a problem hiding this comment.
the issue isnt type(result), its type(comps). Categorical has a isin method, but general ExtensionArray does not
There was a problem hiding this comment.
ok, is there a way to avoid the type ignore, i hate these things because we never remove them
There was a problem hiding this comment.
what about an isinstance(ABCCategorical) ?
There was a problem hiding this comment.
or ok with a run-time import of Categorical if the above doesn't work;
There was a problem hiding this comment.
mypy doesnt recognize any of our ABCFoos
There was a problem hiding this comment.
so you have an aversion to "# type: ignore" and i have an aversion to a runtime import just for mypy. is there a middle ground somewhere? maybe revisit in a few months to see if the type ignore can be removed?
There was a problem hiding this comment.
ok, let's do that, also let's create an issue to see if we can fix the ABC's and mypy
jreback
added
Categorical
|
gentle ping; this is a blocker for tightening types (and avoiding unwanted code patterns) in Block.putmask and Block.where |
|
thanks, i think we need a whatsnew note as well, pls do in a followon |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffxref #32414.
cc @TomAugspurger ATM the new test implemented in tests.arrays.categorical.test_missing is failing on the ndarray case for reasons that I dont fully grok. Are you familiar with this?