DEPR: deprecate default of skipna=False in infer_dtype
State: closed

- [x] follows up on the changed in #17066
- [x] tests adapted / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

I know that `v.0.22` is generally not counted towards the "3 major releases between warning and deprecation" policy, but since this is a private method, I'm thinking it should be OK?

Would make life easier in a couple of places (especially in cleaning up some of the casting code).

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions

Comments:
Hello @h-vetinari! Thanks for updating the PR.









Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on January 04, 2019 at 11:14 Hours UTC
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=h1) Report
> Merging [#24050](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/35b170258e4bb0557b145218b66112dd27aba485?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `60%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24050/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24050      +/-   ##
==========================================
- Coverage   42.45%   42.44%   -0.01%     
==========================================
  Files         161      161              
  Lines       51561    51561              
==========================================
- Hits        21888    21886       -2     
- Misses      29673    29675       +2
```

| Flag | Coverage Δ | |
|---|---|---|
| #single | `42.44% <60%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/integer.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVnZXIucHk=) | `37.97% <0%> (ø)` | :arrow_up: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `48.25% <0%> (-0.17%)` | :arrow_down: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `55.93% <100%> (ø)` | :arrow_up: |
| [pandas/core/algorithms.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWxnb3JpdGhtcy5weQ==) | `50.47% <100%> (ø)` | :arrow_up: |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `75.9% <0%> (-0.26%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=footer). Last update [35b1702...2c9e2ea](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=h1) Report
> Merging [#24050](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/6249355b853878ff06ad8d2affe00d3b629a36ea?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/24050/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24050      +/-   ##
==========================================
+ Coverage   92.38%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52490    52490              
==========================================
+ Hits        48493    48494       +1     
+ Misses       3997     3996       -1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.81% <100%> (ø)` | :arrow_up: |
| #single | `43.04% <0%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.26% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/array\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2FycmF5Xy5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/24050/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `88.09% <0%> (+0.09%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=footer). Last update [6249355...03c46cd](https://codecov.io/gh/pandas-dev/pandas/pull/24050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Hmm...you do have to try a little just to get to the function via Python, so I'm leaning towards ok, but not 100% certain though.

cc @jreback 
I don't think this was ever deprecated. So not really possible to actually switch it now. If you want to add a deprecation warning if its not passed (e.g. make the default ``None``) that would be ok.
@jreback 
The current docstring (from #17066) says "The default of `False`  will be deprecated in a later version of pandas."

Also, it's a method in a module that's explicitly marked as private (big red box at https://pandas.pydata.org/pandas-docs/stable/api.html), so I wonder if deprecation is necessary at all?
What happens when you have 

```
df = pd.DataFrame([[None],[None]],dtype='object')
```

IIRC None is an N/A type.
@bashtage 
> What happens when you have
`df = pd.DataFrame([[None],[None]],dtype='object')`
IIRC None is an N/A type.

This also doesn't change between the two variants:
```
>>> import pandas as pd
>>> import pandas._libs.lib as lib
>>> df = pd.DataFrame([[None], [None]], dtype=object)
>>>
>>> lib.infer_dtype(df, skipna=True)
'integer'
>>> lib.infer_dtype(df.dropna(), skipna=False)
'integer'
```
That said, the result is a bit unexpected, and seems to be due the 2d case being handled differently. In any case, both variants (what you commented on, resp. what I'm changing it to) coincide also for 1d here:
```
>>> lib.infer_dtype(df.values.ravel(), skipna=True)
'empty'
>>> lib.infer_dtype(df.dropna().values.ravel(), skipna=False)
'empty'
>>>
>>> # and for comparison
>>> lib.infer_dtype(df.values.ravel(), skipna=False)
'mixed'
```
@TomAugspurger

I implemented the DeprecationWarning like you requested - this should be ready.
@jreback Merged and green.
@jreback 
Split off a precursor as you asked in https://github.com/pandas-dev/pandas/pull/24050/files#r243769223 / https://github.com/pandas-dev/pandas/pull/24050/files#r243769260: #24560 
@jreback @TomAugspurger 

Merged in #24560 and restored the state that was approved by @TomAugspurger (in https://github.com/pandas-dev/pandas/pull/24050/commits/1ea21fef39c46ecdc628e1be0a6150f05d1c9693), which is all green now. The philosophy of that was to use `skipna=True` in as many places as possible, not least since the original approach of this PR was to set the default to `True` right away.

To avoid getting bogged down in discussing the implications before the cut-off, I've added the last commit, which maintains the current behaviour (`skipna=False`), and only adds the deprecation. The usages within tests are an exception to this, because they are ipso facto correct with a passing CI.

If changing the defaults in the codebase is desired, I can make a follow-up with https://github.com/pandas-dev/pandas/pull/24050/commits/1ea21fef39c46ecdc628e1be0a6150f05d1c9693.
@jreback 
Thanks. Any comment on whether you'd like me to transition the calls from `skipna=False` to `skipna=True` where possible (https://github.com/pandas-dev/pandas/pull/24050#issuecomment-451123022)?
thanks @h-vetinari 

I think we should only change to True in non-trivial cases, IOW the trivial ones can stay False; I think should be very explicit and deliberate for True. (so a lot of the string like ones this makes sense)
A bit late to the party here :-), but:

1) What is actually the rationale for this change?

  I went quickly through the PR here but didn't directly find any reasoning apart from "Would make life easier in a couple of places (especially in cleaning up some of the casting code)." in the top post. Was this discussed in another issue?

  Also to quote Jeff from his last comment just above: "IOW the trivial ones can stay False; I think *should be very explicit and deliberate for True*." (my emphasis) 
  If you want to be explicit in doing True, why then making this the default?

  For the string case I think it certainly makes sense to have this as default, but for the integer case you now get a inconsistency between what `infer_dtype` tells you and what `Series` does: `infer_dtype([1, 2, np.nan] == 'integer` vs `pd.Series([1, 2, np.nan]) == float64` (of course, if we ever have NA support by default for ints, this argument is no longer valid).

2) If we keep the warning (not really a strong opinion here, mainly wondering the rationale in the above comment): what do you all think of making this a DeprecationWarning instead of FutureWarning first, given that it is more a functions used by developers / other libraries and less directly by users? 

  
@jorisvandenbossche 
> 1. What is actually the rationale for this change?

Actually, my main reason for deprecating was that it has been "announced" in the docstring since 0.21 (see diff of this PR):
```
The default of ``False`` will be deprecated in a later version of pandas.
```

That being said, I think the inference generally makes more sense when nans are ignored (just that historically, `skipna` was not available until #17066).

> 2. If we keep the warning (not really a strong opinion here, mainly wondering the rationale in the above comment): what do you all think of making this a DeprecationWarning instead of FutureWarning first, given that it is more a functions used by developers / other libraries and less directly by users?

I agree that it's not very user-facing. I have no strong preference for the type of warning.