Skip to content
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.

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

BUG: Convert uint64 in maybe_convert_objects #14916

Closed

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Dec 19, 2016

Adds handling for uint64 objects during conversion. When negative numbers and uint64 are detected, we then convert the result to object.

Picks up where #4845 left off. Closes #4471.

@codecov-io
Copy link

codecov-io commented Dec 19, 2016

Current coverage is 84.64% (diff: 100%)

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

@@             master     #14916   diff @@
==========================================
  Files           144        144          
  Lines         51016      51016          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43184      43185     +1   
+ Misses         7832       7831     -1   
  Partials          0          0          

Powered by Codecov. Last update 3ccb501...ed325cd

try:
ints[i] = val
except OverflowError:
seen_uint = seen_uint or (val > np.iinfo(np.int64).max)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would pull set np.iinfo(np.int64).max as a constant in the module.

Copy link
Member Author

@gfyoung gfyoung Dec 19, 2016

Choose a reason for hiding this comment

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

Done.

@@ -234,6 +234,21 @@ def test_empty_like(self):
self._check_behavior(arr, expected)


def test_maybe_convert_objects_uint64():
Copy link
Contributor

Choose a reason for hiding this comment

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

these tests are all in

pandas.types.test_inference

Copy link
Member Author

@gfyoung gfyoung Dec 19, 2016

Choose a reason for hiding this comment

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

Done.

@jreback jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions labels Dec 19, 2016
@@ -251,3 +251,4 @@ Bug Fixes


- Require at least 0.23 version of cython to avoid problems with character encodings (:issue:`14699`)
- Bug in ``lib.maybe_convert_objects()`` in which unsigned 64-bit integers were not being properly converted (:issue:`4471`)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not public. I would refer to this more generically (maybe also construct a test which actually hits this in practice from a user perspective)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough. Done.

@gfyoung gfyoung closed this Dec 19, 2016
@gfyoung gfyoung deleted the convert-objects-uint64 branch December 19, 2016 18:17
@gfyoung gfyoung restored the convert-objects-uint64 branch December 19, 2016 18:18
@gfyoung gfyoung reopened this Dec 19, 2016
@gfyoung gfyoung force-pushed the convert-objects-uint64 branch 3 times, most recently from edb04d3 to 47de0df Compare December 19, 2016 21:01
Adds handling for uint64 objects during conversion.
When negative numbers and uint64 are detected, we
then convert the result to object.

Picks up where pandas-devgh-4845 left off. Closes pandas-devgh-4471.
@jreback jreback added this to the 0.20.0 milestone Dec 20, 2016
@jreback jreback closed this in 0c52813 Dec 20, 2016
@jreback
Copy link
Contributor

jreback commented Dec 20, 2016

thanks!

@gfyoung gfyoung deleted the convert-objects-uint64 branch December 20, 2016 14:39
ShaharBental pushed a commit to ShaharBental/pandas that referenced this pull request Dec 26, 2016
Adds handling for `uint64` objects during conversion.  When negative
numbers and `uint64` are detected, we then convert the result to
`object`.    Picks up where pandas-dev#4845 left off. Closes pandas-dev#4471.

Author: gfyoung <gfyoung17@gmail.com>

Closes pandas-dev#14916 from gfyoung/convert-objects-uint64 and squashes the following commits:

ed325cd [gfyoung] BUG: Convert uint64 in maybe_convert_objects
jreback pushed a commit that referenced this pull request Jan 17, 2017
1) Introduces and propagates `UInt64Index`, an index specifically for
`uint`.  xref #14935    2) <strike> Patches bug from #14916 that makes
`maybe_convert_objects` robust against the known `numpy` bug that
`uint64` cannot be compared to `int64`.  This bug was caught during
testing of `UInt64Index`. </strike>    **UPDATE**: Patched in #14951

Author: gfyoung <gfyoung17@gmail.com>

Closes #14937 from gfyoung/create-uint64-index and squashes the following commits:

8ab6fbd [gfyoung] ENH: Create and propagate UInt64Index
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request Mar 21, 2017
1) Introduces and propagates `UInt64Index`, an index specifically for
`uint`.  xref pandas-dev#14935    2) <strike> Patches bug from pandas-dev#14916 that makes
`maybe_convert_objects` robust against the known `numpy` bug that
`uint64` cannot be compared to `int64`.  This bug was caught during
testing of `UInt64Index`. </strike>    **UPDATE**: Patched in pandas-dev#14951

Author: gfyoung <gfyoung17@gmail.com>

Closes pandas-dev#14937 from gfyoung/create-uint64-index and squashes the following commits:

8ab6fbd [gfyoung] ENH: Create and propagate UInt64Index
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Dtype Conversions Unexpected or buggy dtype conversions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants