BUG: Coerce to numeric despite uint64 conflict #17823
Conversation
16bf62e to
0904a55
Compare
Codecov Report
@@ Coverage Diff @@
## master #17823 +/- ##
==========================================
- Coverage 91.26% 91.24% -0.02%
==========================================
Files 163 163
Lines 49980 49980
==========================================
- Hits 45613 45604 -9
- Misses 4367 4376 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17823 +/- ##
==========================================
- Coverage 91.24% 91.22% -0.02%
==========================================
Files 163 163
Lines 49992 49992
==========================================
- Hits 45613 45604 -9
- Misses 4379 4388 +9
Continue to review full report at Codecov.
|
| (np.array([2**63, np.nan], dtype=object), set()), | ||
| (np.array([str(2**63), np.nan], dtype=object), set()), | ||
| (np.array([np.nan, 2**63], dtype=object), set()), | ||
| (np.array([np.nan, str(2**63)], dtype=object), set())]) |
There was a problem hiding this comment.
is the na_values part of the parametrize needed since it is all the same empty set ?
There was a problem hiding this comment.
Doh! Good catch. Will fix.
| @@ -39,6 +39,11 @@ | |||
| from pandas.util import testing as tm | |||
|
|
|||
|
|
|||
| @pytest.fixture(params=[True, False], ids=lambda val: str(val)) | |||
| def coerce(request): | |||
| return request.param | |||
There was a problem hiding this comment.
Should we do this as a parameterize instead of fixture? (I don't know whether are some 'rules' where it is better to use one or the other, but in this case it really seems to be a parameterizing of the test, so I would find it easier to read the code if it uses paramatrize)
There was a problem hiding this comment.
I use fixture whenever I find myself using the same parametrization multiple times. As you can see below, I use it three times.
| # see gh-17007 and gh-17125 | ||
| # | ||
| # Still returns float despite the uint64-nan conflict, | ||
| # which would normally force the casting to object. |
There was a problem hiding this comment.
can you also test with errors='raise' and ignore here
0904a55 to
93e52e9
Compare
|
@jreback @jorisvandenbossche : All is green. PTAL. |
| return True | ||
|
|
||
| if self.null_: | ||
| msg = ("uint64 array detected, and such an " |
There was a problem hiding this comment.
interesting, so these cases were not actually tested then? (IOW you didn't have to fix any tests)?
There was a problem hiding this comment.
nvm. I see you blew away the entire section and re-wrote.
|
thanks! |
Previously,
to_numericwas not coercing elements to numeric if the conversion was going to be lossy (e.g.uint64combined withnan), even whenerrors='coerce'. Now theerrorsparameter takes precedence.Closes #17007.
Closes #17125.