BUG: Convert uint64 in maybe_convert_numeric
State: closed

Add handling for `uint64` elements in an array with the follow behavior specifications:

1) If `uint64` and `NaN` are both detected, the original input will be returned if `coerce_numeric`
is `False`. Otherwise, an `Exception` is raised.

2) If `uint64` and negative numbers are both detected, the original input be returned if `coerce_numeric` is `False`. Otherwise, an `Exception` is raised.

Closes #14982.
Partial fix for #14983.

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

Comments:
Sadly, this function gets even more complex because of all the different ways in which you can hit those two behavior cases I mentioned.  In addition, you have to check those cases in so many different places (hence my decision to write two inner functions to avoid duplicate code).  If there are ways in which the logic can be simplified without sacrificing robustness, that would be great!
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/15005?src=pr) is 84.78% (diff: 100%)
> Merging [#15005](https://codecov.io/gh/pandas-dev/pandas/pull/15005?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will not change coverage

```diff
@@             master     #15005   diff @@
==========================================
  Files           145        145          
  Lines         51090      51090          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits          43315      43315          
  Misses         7775       7775          
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [0d3ecfa...c3bd28a](https://codecov.io/gh/pandas-dev/pandas/compare/0d3ecfa8ba3930b688a6781c7f84138bc80da41a...c3bd28ae0ad89b75a702920f76dfe8b9dd1931ac?src=pr)
try this (you can't do this directly on inference.pyx because its only *part* of a file (IOW it doesn't have all of the proper declarations), but lib.pyx does (which is the congolomeration).
```
(pandas) bash-3.2$ cython -a pandas/lib.pyx -I pandas/src
(pandas) bash-3.2$ git st
On branch weightby
Your branch is ahead of 'agg2' by 2 commits.
  (use "git push" to publish your local commits)
Untracked files:
  (use "git add <file>..." to include in what will be committed)

        pandas/inference.html
        pandas/lib.html
        pandas/properties.html
        pandas/reduce.html

nothing added to commit but untracked files present (use "git add" to track)
```

at the top of lib.pyx, you can recompile with this = True, then %prun works nicely
```
# cython: profile=False
```
@jreback : Ran `cython -a` as requested, and no hotspots to report.  Your `cdef inline bint` suggestion really made a difference as you already said. :smile:

Some timing info (note I setup the array so that we hit as many of the potential pain points as possible):
~~~python
In [1]: from pandas import lib, np
In [2]: n = 1000000
In [3]:
In [4]: arr = np.repeat([2**63], n)
In [5]: arr = arr + np.arange(1000000).astype('uint64')
In [6]: arr = np.array([arr[i] if i%2 == 0 else str(arr[i]) for i in range(n)],
                        dtype=object)
In [7]:
In [8]: arr[-1] = -1
In [9]: %timeit -n 100 lib.maybe_convert_objects(arr, set())
100 loops, best of 3: 44 µs per loop
~~~
can u add an asv to that effect?
@jreback : Added `asv` test.  Ready to merge if there are no other concerns.
@gfyoung You updated `maybe_convert_numeric` to work with uints, but `maybe_convert_numeric` is used in `to_numeric`, so the result is also that `to_numeric` now can handle those? 
If so, can you add tests for this as well? (as this is the API for users) 

Can you also show the results of the asv tests for `to_numeric` before/after to check the perf impact.

I was wondering, regarding your question on code complexity, you now call the `_check_uint64_nan` and `_check_uint64_int64_conflict` in many places throughout the for loop. But could it also be a choice to only call those *after* the for loop? This would give worse performance for cases with conflicts (as the original values are not returned / error raised on first occurence of a conflict, but only after parsing all values), but would improve code complexity slightly and give possibly slightly better performance for cases without conflicts (for arrays that actually get converted)
@jorisvandenbossche : To address your points:
1) `to_numeric` : Yes!  It can handle `uint64` now.  I will add test cases.
2) Checking at the end certainly is plausible.  Given how fast it is, it couldn't hurt.
3) Benchmarks for `to_numeric` can be provided once (1) and (2) have been resolved.

For (2) and (3), I don't expect perf to be impacted too much, but we'll see.
@jorisvandenbossche : I just checked (1), and it appears more work will be needed (xref #14901).  Could we put that off as a follow-up?  I can tackle (2) and (3) for this PR.
thanks!

so if you follow up with fixes for ``.to_numeric`` and change this to a state machine (e.g. really a class with methods), then this would be great.