Skip to content

BUG: Fix maybe_convert_numeric for unhashable objects #13326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

BUG: Fix maybe_convert_numeric for unhashable objects #13326

wants to merge 1 commit into from

Conversation

RogerThomas
Copy link
Contributor

@RogerThomas RogerThomas commented May 30, 2016 •
edited
Loading

@RogerThomas RogerThomas force-pushed the fix_maybe_convert_numeric_for_unhashable_objects branch from 956f9e3 to 80b8515 Compare May 30, 2016 16:59
jreback
jreback reviewed May 30, 2016
View reviewed changes
pandas/tests/test_infer_and_convert.py
@@ -102,6 +102,12 @@ def test_scientific_no_exponent(self):
result = lib.maybe_convert_numeric(arr, set(), False, True)
self.assertTrue(np.all(np.isnan(result)))

def test_convert_non_hashable(self):
arr = np.array([[10.0, 2], 1.0, 'apple'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add the issue number here as a comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test usage of pd.to_numeric as well (tests in pandas/tools

@codecov-io
Copy link

codecov-io commented May 30, 2016 •
edited
Loading

Current coverage is 84.22%

Merging #13326 into master will increase coverage by <.01%

@@             master     #13326   diff @@
==========================================
  Files           138        138          
  Lines         50684      50681     -3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          42684      42682     -2   
+ Misses         8000       7999     -1   
  Partials          0          0

Powered by Codecov. Last updated by c0850ea...80b8515

@jreback jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions labels May 30, 2016
jreback
jreback reviewed May 30, 2016
View reviewed changes
doc/source/whatsnew/v0.18.2.txt
@@ -317,6 +317,7 @@ Bug Fixes
- Bug in ``groupby`` where ``apply`` returns different result depending on whether first result is ``None`` or not (:issue:`12824`)


- Bug in ``pd.to_numeric`` when ``errors='coerce'`` returns same dataframe/series when dataframe/series contains non-hashable objects (:issue:`13324`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug in pd.to_numeric when errors='coerce' and input contains non-hashable objects....

@RogerThomas RogerThomas force-pushed the fix_maybe_convert_numeric_for_unhashable_objects branch from 8fc536f to 2db755f Compare May 31, 2016 09:12
@RogerThomas
Copy link
Contributor Author

@jreback i've made the changes you suggested

jreback
jreback reviewed May 31, 2016
View reviewed changes
pandas/tests/test_infer_and_convert.py
# Test for Bug #13324
arr = np.array([[10.0, 2], 1.0, 'apple'])
result = lib.maybe_convert_numeric(arr, set(), False, True)
np.testing.assert_array_equal(result, np.array([np.nan, 1.0, np.nan]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use tm.assert_numpy_array_equal (we don't use th numpy testing functions at all; in fact there is a check again them; though this one slipped thru)

@RogerThomas RogerThomas force-pushed the fix_maybe_convert_numeric_for_unhashable_objects branch from 752ac47 to bf4f9d8 Compare May 31, 2016 13:28
@jreback
Copy link
Contributor

jreback commented May 31, 2016

looks good. ping when green.

@jreback jreback added this to the 0.18.2 milestone May 31, 2016
@RogerThomas RogerThomas force-pushed the fix_maybe_convert_numeric_for_unhashable_objects branch from b09a1b3 to 76a0738 Compare May 31, 2016 13:29
@jreback jreback closed this in f3d7c18 May 31, 2016
@jreback
Copy link
Contributor

jreback commented May 31, 2016 •
edited
Loading

@RogerThomas thanks!

I was just experimenting with something and:

so maybe should change this to if getattr(val, '__hash__', None) is not None

though can't seem to get this to trigger

In [1]: 'a'.__hash__
Out[1]: <method-wrapper '__hash__' of str object at 0x1002e1508>

In [2]: None.__hash__
Out[2]: <method-wrapper '__hash__' of NoneType object at 0x100180c40>

In [3]: 1.__hash__
  File "<ipython-input-3-e97b483155ff>", line 1
    1.__hash__
             ^
SyntaxError: invalid syntax

In [5]: [].__hash__ is None
Out[5]: True

@jreback
Copy link
Contributor

jreback commented May 31, 2016

if you want to see if you can generate a test that makes this fail (not sure). If so , can you do a followup?

@RogerThomas
Copy link
Contributor Author

Hey @jreback, I think this is just due to python's syntax parser.

In [1]: a = 1

In [2]: a.__hash__
Out[2]: <method-wrapper '__hash__' of int object at 0xa63b00>

I don't think this could ever fail

@jreback
Copy link
Contributor

jreback commented May 31, 2016

oh ok that makes sense thanks

@RogerThomas RogerThomas deleted the fix_maybe_convert_numeric_for_unhashable_objects branch June 1, 2016 13:46
@jorisvandenbossche jorisvandenbossche mentioned this pull request Sep 20, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Dtype Conversions Unexpected or buggy dtype conversions
Projects
None yet
Milestone
0.19.0
Development

Successfully merging this pull request may close these issues.

to_numeric(errors='coerce') broken when using unhashable objects
3 participants
@RogerThomas @codecov-io @jreback