ENH: Add skipna parameter to infer_dtype
State: closed

 closes #17059 
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master -u -- "*.py" | flake8 --diff``
 - [x] whatsnew entry

Labels:
name: Enhancement
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate

Comments:
Failures related to `pyarrow` or `feather` are unrelated to this PR.
actually I am not big on this. This is just a bug fix, we should be ignoring NaN when we have a string,unciode,bytes objects. otherwise.
@jreback 

> actually I am not big on this. This is just a bug fix, we should be ignoring NaN when we have a string,unciode,bytes objects. otherwise.

I don't really follow what you're saying here. Can you clarify?
> actually I am not big on this. This is just a bug fix, we should be ignoring NaN when we have a string,unciode,bytes objects. otherwise.

Why is this a bug-fix?  The behavior is consistent with what I would expect because there is `NaN` and other elements in the array-like.  I don't see why we should be changing this behavior that drastically.
it should mirror what we already do for other types. We automatically skip na's; they don't have an impact on the inference type. In fact that's the entire point of this routine.

```
In [4]: infer_dtype([pd.Timestamp('20130101'), np.nan])
Out[4]: 'datetime'

In [5]: infer_dtype(['foo', np.nan])
Out[5]: 'mixed'

In [6]: infer_dtype([pd.Timestamp('20130101')])
Out[6]: 'datetime'

In [7]: infer_dtype([pd.Timestamp('20130101'), pd.NaT])
Out[7]: 'datetime'

In [8]: infer_dtype(['foo'])
Out[8]: 'string'

In [9]: infer_dtype(['foo', np.nan])
Out[9]: 'mixed'

```
Right. That's where we want to be, but this routine has existed for a very long time with this behavior and I don't think it'd be a great idea to break it right this second. I think @gfyoung had the right idea with 1 or more deprecation cycles before changing the default behavior to skip NA values by default.
> Right. That's where we want to be, but this routine has existed for a very long time with this behavior and I don't think it'd be a great idea to break it right this second. I think @gfyoung had the right idea with 1 or more deprecation cycles before changing the default behavior to skip NA values by default.

this is broken. I don't really have a problem with adding a ``skipna``,  though adds a lot more logic. The problem is these are now different for different dtypes. I don't think easy to reconcile this.



> I don't think easy to reconcile this.

Wait for my refactor, which reconciles them nicely.
>Wait for my refactor, which reconciles them nicely.

I figured there would be a way. :smile: 
I'll put up another PR with the refactor, but since it's quite a large refactor and may introduce a perf regression let's leave this one open as well in case the refactor isn't viable from a perf perspective.
@cpcloud : You could have made a separate commit here and just show `asv` comparisons between them.
Indeed. I'll do that. Thanks for the tip!
@cpcloud : Rebase onto `master` so that `pyarrow` tests don't fail anymore
cool, will do once the benchmarks are finished!
asv results (top offenders):

```
    before     after       ratio                                                                                                                                                               
  [c55dbf06] [04003c8d]                                                                                                                                                                        
!  117.73ms     failed       n/a  frame_ctor.FrameConstructorDTIndexFromOffsets.time_frame_ctor('FY5253Quarter_2', 2)                                                                          
+  204.01ms   336.96ms      1.65  inference.to_numeric_downcast.time_downcast('string-nint', 'unsigned')                                                                                       
+  197.91ms   314.76ms      1.59  inference.to_numeric_downcast.time_downcast('string-int', 'integer')                                                                                         
+  189.44ms   298.65ms      1.58  inference.to_numeric_downcast.time_downcast('string-int', None)                                                                                              
+  105.50ms   160.92ms      1.53  packers.packers_read_sas.time_read_sas7bdat                                                                                                                  
+   33.91ms    50.75ms      1.50  groupby.GroupBySuite.time_diff('float', 100)                                                                                                                 
+  213.73ms   305.88ms      1.43  inference.to_numeric_downcast.time_downcast('string-nint', 'signed')                                                                                         
+  196.81ms   276.27ms      1.40  inference.to_numeric_downcast.time_downcast('string-nint', None) 
```
It's possible that first one is failing because I haven't yet run the entire test suite to make sure I haven't broken anything outside of the inference code.
Yep, some failing tests :) Fixing them up right now
Here are the inference benchmarks with latest commit:

```
    before     after       ratio
  [1d0e6a15] [390aae13]
+  115.00μs   164.03μs      1.43  inference.to_numeric.time_from_str_ignore
+  500.56μs   706.92μs      1.41  inference.DtypeInfer.time_uint32
+  149.24ms   195.31ms      1.31  inference.to_numeric_downcast.time_downcast('string-float', 'float')
+  222.45ms   279.17ms      1.25  inference.to_numeric_downcast.time_downcast('string-int', None)
+  157.01ms   196.73ms      1.25  inference.to_numeric_downcast.time_downcast('string-float', 'integer')
+  196.16ms   244.83ms      1.25  inference.to_numeric_downcast.time_downcast('string-int', 'signed')
+  166.39ms   202.27ms      1.22  inference.to_numeric_downcast.time_downcast('string-float', 'signed')
+   15.05ms    18.08ms      1.20  inference.DtypeInfer.time_datetime64
+  193.79ms   232.27ms      1.20  inference.to_numeric_downcast.time_downcast('string-nint', None)
+  168.74ms   201.07ms      1.19  inference.to_numeric_downcast.time_downcast('string-float', 'unsigned')
+   46.88ms    55.18ms      1.18  inference.to_numeric_downcast.time_downcast('int-list', 'integer')
+    2.69ms     3.16ms      1.17  inference.to_numeric_downcast.time_downcast('datetime64', 'float')
+   27.13ms    31.86ms      1.17  inference.to_numeric_downcast.time_downcast('datetime64', 'signed')
+  169.50ms   194.19ms      1.15  inference.to_numeric_downcast.time_downcast('string-float', None)
+   49.51ms    56.34ms      1.14  inference.to_numeric_downcast.time_downcast('int-list', 'unsigned')
+    1.34ms     1.51ms      1.13  inference.to_numeric_downcast.time_downcast('datetime64', None)
+   36.23ms    40.06ms      1.11  inference.to_numeric_downcast.time_downcast('int-list', 'float')
-   39.59μs    35.49μs      0.90  inference.to_numeric.time_from_float
-  770.33ms   688.99ms      0.89  inference.MaybeConvertNumeric.time_convert
-   19.51ms    17.18ms      0.88  inference.to_numeric.time_from_str_coerce
-    6.96ms     6.05ms      0.87  inference.DtypeInfer.time_int64
-  467.61ms   401.44ms      0.86  inference.to_numeric.time_from_numeric_str
```
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@9e6bb42`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff            @@
##             master   #17066   +/-   ##
=========================================
  Coverage          ?   91.01%           
=========================================
  Files             ?      161           
  Lines             ?    49363           
  Branches          ?        0           
=========================================
  Hits              ?    44927           
  Misses            ?     4436           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.78% <ø> (?)` | |
| #single | `40.25% <ø> (?)` | |


------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17066?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/17066?src=pr&el=footer). Last update [9e6bb42...a6d122b](https://codecov.io/gh/pandas-dev/pandas/pull/17066?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/17066?src=pr&el=h1) Report
> Merging [#17066](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/793020293ee1e5fa023f45c12943a4ac51cc23d0?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `n/a`.

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

```diff
@@            Coverage Diff             @@
##           master   #17066      +/-   ##
==========================================
+ Coverage   91.02%   91.03%   +<.01%     
==========================================
  Files         161      161              
  Lines       49393    49396       +3     
==========================================
+ Hits        44962    44968       +6     
+ Misses       4431     4428       -3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.8% <ø> (+0.02%)` | :arrow_up: |
| #single | `40.28% <ø> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.66% <0%> (-0.1%)` | :arrow_down: |
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.44% <0%> (ø)` | :arrow_up: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.05% <0%> (+1.81%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17066?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/17066?src=pr&el=footer). Last update [7930202...1883927](https://codecov.io/gh/pandas-dev/pandas/pull/17066?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

Pushed up with `inline` added. Perf improved a bit on the `inference` benchmarks:

```
    before     after       ratio
  [9e6bb42f] [a6d122b9]
+  195.64ms   225.73ms      1.15  inference.to_numeric_downcast.time_downcast('string-int', None)
+    2.15ms     2.44ms      1.13  inference.DtypeInfer.time_int32
+  201.92ms   224.39ms      1.11  inference.to_numeric_downcast.time_downcast('string-int', 'float')
+  371.52ms   409.00ms      1.10  inference.to_numeric.time_from_numeric_str
-  263.91ms   238.96ms      0.91  inference.to_numeric_downcast.time_downcast('string-nint', 'signed')
-   16.77ms    15.12ms      0.90  inference.DtypeInfer.time_datetime64
-  228.10ms   199.98ms      0.88  inference.to_numeric_downcast.time_downcast('string-nint', None)
-  242.66ms   212.45ms      0.88  inference.to_numeric_downcast.time_downcast('string-int', 'integer')
-  232.26ms   202.74ms      0.87  inference.to_numeric_downcast.time_downcast('string-nint', 'float')
-  165.75ms   141.11ms      0.85  inference.to_numeric_downcast.time_downcast('string-float', 'unsigned')
-  187.46ms   142.09ms      0.76  inference.to_numeric_downcast.time_downcast('string-float', 'float')
```

I'll run the full suite again to make sure I'm not introducing any massive regressions elsewhere.
@jreback @gfyoung No big regressions :)

Here's everything that was worse than 1x `upstream/master`:

```
    before     after       ratio                                                                                                                                                               
  [9e6bb42f] [a6d122b9]                                                                                                                                                                        
+  794.88ms      1.13s      1.42  packers.Excel.time_write_excel_openpyxl                                                                                                                      
+    3.80ms     5.03ms      1.33  strings.StringMethods.time_get                               
+  211.56ms   276.62ms      1.31  inference.to_numeric_downcast.time_downcast('string-nint', 'float')
+   16.39ms    21.34ms      1.30  gil.nogil_read_csv.time_read_csv_object                      
+   23.70ms    30.14ms      1.27  binary_ops.Ops.time_frame_mult(True, 'default')                               
+    3.17ms     3.82ms      1.21  strings.StringMethods.time_upper                             
+    4.30ms     5.14ms      1.20  strings.StringMethods.time_rstrip                            
+    6.27ms     7.50ms      1.20  frame_methods.Reindex.time_reindex_upcast                                 
+  164.44ms   193.30ms      1.18  inference.to_numeric_downcast.time_downcast('string-float', 'unsigned')
+    5.56ms     6.48ms      1.17  categoricals.Categoricals.time_union                                      
+    4.09ms     4.71ms      1.15  strings.StringMethods.time_lstrip                            
+    6.23ms     7.17ms      1.15  algorithms.Algorithms.time_add_overflow_pos_arr                                                                                                              
+   28.19ms    32.39ms      1.15  stat_ops.stats_rank_pct_average_old.time_stats_rank_pct_average_old
+  336.85ns   382.73ns      1.14  attrs_caching.DataFrameAttributes.time_get_index                        
+    1.01ms     1.14ms      1.13  join_merge.join_non_unique_equal.time_join_non_unique_equal  
+    4.23ms     4.75ms      1.12  strings.StringMethods.time_strip                                            
+   14.70ms    16.52ms      1.12  binary_ops.Ops2.time_frame_int_mod                                                                                                                           
+     6.83s      7.67s      1.12  gil.nogil_datetime_fields.time_datetime_field_day                 
+   17.80ms    19.91ms      1.12  parser_vb.read_csv1.time_sep                                         
+    2.70ms     3.01ms      1.12  groupby.GroupBySuite.time_size('float', 10000)               
+    3.09ms     3.44ms      1.11  strings.StringMethods.time_contains_many_noregex                                                                                                             
+    2.78ms     3.07ms      1.10  period.Algorithms.time_drop_duplicates_pseries               
+  253.54ms   279.28ms      1.10  inference.to_numeric_downcast.time_downcast('string-nint', 'unsigned')       
+   83.70ms    92.18ms      1.10  eval.Eval.time_mult('numexpr', 'all')                                           
+   21.67ms    23.86ms      1.10  frame_methods.frame_mask_bools.time_frame_mask_bools                
+   15.72ms    17.30ms      1.10  categoricals.Categoricals.time_constructor_regular  
```
@cpcloud : Nice!  Great to see that the refactoring worked out.
@jreback this is ready to go
Actually one sec.
> Actually one sec.

I presume you meant to push that change first?
@gfyoung Yep! Pushed.
Given that both @jreback and I have approved, and you're ready to merge this, I imagine three approvals from ">= core-committers" makes this merge-able on green :smile: 
@gfyoung agree :) Merging on green.
Thanks @cpcloud !