BUG: Fix #13149 and ENH: 'copy' param in Index.astype() #13209

pijucha · 2016-05-17T21:36:03Z

closes NaNs in Float64Index are converted to silly integers using index.astype('int') #13149
tests added / passed
passes git diff upstream/master | flake8 --diff
whatsnew entry

Update

Float64Index.astype(int) raises ValueError if a NaN is present.
Previously, it converted NaN's to the smallest negative integer.
TimedeltaIndex.astype(int) and DatetimeIndex.astype(int) return.
Int64Index, which is consistent with behavior of other Indexes.
Previously, they returned a numpy.array of ints.
Added:
- bool parameter 'copy' to Index.astype()
- shared doc string to .astype()
- tests on .astype() (consolidated and added new)
- bool parameter 'copy' to Categorical.astype()
Fixed core.common.is_timedelta64_ns_dtype().
Set a default NaT representation to a string type in a parameter of DatetimeIndex._format_native_types().
Previously, it produced a unicode u'NaT' in Python2.

jreback · 2016-05-17T21:51:36Z

tests pls.

pijucha · 2016-05-17T22:29:30Z

Added tests. (I hope I'm doing this right.)

jreback · 2016-05-17T22:34:08Z

pandas/tests/indexes/test_numeric.py

add tests for datetimelikes as well to cover the bases; I think your change might be changing behavior there as well (which is not correct)

Do you mean this conversion:

pd.DatetimeIndex([1,2,np.nan]).astype(int) Out[16]: array([ 1, 2, -9223372036854775808])

Its behaviour is unchanged. I didn't touch other indexes.

yes - just want to make sure that is tested as well

Done.

(BTW, DatetimeIndex.astype(int), TimedeltaIndex.astype(int) return a numpy.array. Isn't it a bug? PeriodIndex and Float64Index return Int64Index.)

jreback · 2016-05-18T01:21:07Z

pandas/tests/indexes/test_datetimelike.py

that is wrong ; see if u can fix so these are returned as index types

I've fixed this, I guess.

jreback · 2016-05-18T19:43:33Z

doc/source/whatsnew/v0.18.2.txt

so you can list the second point as a separate bug fix issue and use the PR number as the issue number

jreback · 2016-05-19T18:48:18Z

pandas/tseries/index.py

actually, better to use is_integer_dtype(dtype) more generic her

(and change dtype == np.object_ to is_object_dtype(dtype))
and dtype == _NS_DTYPE to is_int64_dtype(dtype)
and dtype == str to is_string_dtype(dtype)

and dtype == _NS_DTYPE to is_int64_dtype(dtype)

I guess it's is_datetime64_ns_dtype(dtype).

yep - typing fast !

jreback · 2016-05-19T18:49:06Z

looks pretty good. a couple of syntax changes. ping when green.

sinhrks · 2016-05-19T19:57:28Z

doc/source/whatsnew/v0.18.2.txt

Int64Index

pijucha · 2016-05-21T18:34:50Z

pandas/tseries/tests/test_timeseries.py

Here, I ran into an issue with the null Period Period(NaT, 'D") not being seen as null by pd.isnull(). It's also impossible to directly check array or index equivalence if they contain null Periods.

I can elaborate on this and possible fixes (the fixes I tried are rather beyond the scope of this commit). Unless we don't want Period(NaT, 'D") to be treated as null.

see #12759 this will be addressed, so for now use Period('NaT', 'D') until this is fixed.
You can add a TODO / comment with this issue number and can be updated later.

OK. Added TODO.

jreback · 2016-05-21T19:25:56Z

doc/source/whatsnew/v0.18.2.txt

you can remove this; this is an internal function

jreback · 2016-05-21T19:30:24Z

@pijucha looks pretty good.

There are some existing test_astype tests that would like you to consolidate with the new ones

[Sat May 21 15:29:01 ~/pandas]$ grin 'test_astype' pandas

pandas/tseries/tests/test_base.py:
   53 :     def test_astype_str(self):
pandas/tseries/tests/test_period.py:
 1630 :     def test_astype(self):
pandas/tseries/tests/test_timedeltas.py:
 1194 :     def test_astype(self):
pandas/tseries/tests/test_timeseries.py:
 1809 :     def test_astype_object(self):
 2566 :     def test_astype(self):

pijucha · 2016-05-23T00:48:27Z

pandas/tseries/tests/test_timeseries.py

Here, Python2 returns an Index with mixed type: Date '2016-05-16' is a string, and NaT's are unicode. Is it all right?

in py2 these should all be strings not Unicode
never should be mixed

can u show an example?

An example from tests (nb. it failed assert_index_equal with pyhton2.7):

In [2]: idx = DatetimeIndex(['2016-05-16', 'NaT', NaT, np.NaN]) In [3]: idx Out[3]: DatetimeIndex(['2016-05-16', 'NaT', 'NaT', 'NaT'], dtype='datetime64[ns]', freq=None) In [4]: idx2 = idx.astype(str) In [5]: idx2 Out[5]: Index([u'2016-05-16', u'NaT', u'NaT', u'NaT'], dtype='object') In [7]: type(idx2[0]) Out[7]: str In [8]: type(idx2[1]) Out[8]: unicode

I think this u('NaT') may be responsible:

# class DatetimeIndex: def _format_native_types(self, na_rep=u('NaT'), date_format=None, **kwargs):

hmm looks like we are always returning Unicode

we should be returning a string here under py2 (and the display formatters will make them Unicode when displaying)

see if returning as a string breaks anything

Seems to be ok. It's just passed Python2.7 tests with "not network and not slow and not disabled".

ok great. make sure we add this as a test (and comment)

pijucha · 2016-05-23T05:17:14Z

pandas/tseries/index.py

Changed this default na_rep. No breaks observed.

k thanks

I don't know if you added that test (below) which discovered this bug? (if not pls add with a nice comment)

Yes. The test is among astype tests for DatetimeIndex (just an assert_index_equal(), which previously discovered the Indexes had different inferred types).
I'll comment. Or should I rather make it more explicit and place somewhere else?

yep I saw that. its fine

jreback · 2016-05-23T12:35:59Z

@pijucha looks really good!

some test moving around / slight refactoring.

ping when green.

1. Float64Index.astype(int) raises ValueError if a NaN is present. Previously, it converted NaN's to the smallest negative integer. 2. TimedeltaIndex.astype(int) and DatetimeIndex.astype(int) return Int64Index, which is consistent with behavior of other Indexes. Previously, they returned a numpy.array of ints. 3. Added: - bool parameter 'copy' to Index.astype() - shared doc string to .astype() - tests on .astype() (consolidated and added new) - bool parameter 'copy' to Categorical.astype() 4. Internals: - Fixed core.common.is_timedelta64_ns_dtype(). - Set a default NaT representation to a string type in a parameter of DatetimeIndex._format_native_types(). Previously, it produced a unicode u'NaT' in Python2.

pijucha · 2016-05-23T19:40:21Z

@jreback It's green.

jreback · 2016-05-23T20:46:16Z

@pijucha thanks!

very nice PR. you really dove deep into some internals! keep em coming!

pijucha · 2016-05-24T00:32:35Z

@jreback Thanks for the guidelines! They helped a lot.

jreback added Bug Dtype Conversions Unexpected or buggy dtype conversions labels May 17, 2016

pijucha force-pushed the bug13149 branch 2 times, most recently from cb58d56 to ade4e95 Compare May 17, 2016 22:27

jreback reviewed May 17, 2016
View reviewed changes

pijucha force-pushed the bug13149 branch 2 times, most recently from 4d310e9 to 07e27e1 Compare May 18, 2016 00:36

jreback reviewed May 18, 2016
View reviewed changes

pijucha force-pushed the bug13149 branch from 07e27e1 to b5c1843 Compare May 18, 2016 19:35

jreback reviewed May 18, 2016
View reviewed changes

pijucha force-pushed the bug13149 branch 3 times, most recently from 7243bb6 to 3d3a5b3 Compare May 19, 2016 15:50

jreback reviewed May 19, 2016
View reviewed changes

jreback added this to the 0.18.2 milestone May 19, 2016

sinhrks reviewed May 19, 2016
View reviewed changes

doc/source/whatsnew/v0.18.2.txt Outdated

Copy link

Member

sinhrks May 19, 2016

Choose a reason for hiding this comment

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

Int64Index

pijucha reviewed May 21, 2016
View reviewed changes

pijucha force-pushed the bug13149 branch from d6e4465 to d4d5c93 Compare May 21, 2016 18:55

jreback reviewed May 21, 2016
View reviewed changes

doc/source/whatsnew/v0.18.2.txt Outdated

Copy link

Contributor

jreback May 21, 2016

Choose a reason for hiding this comment

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

you can remove this; this is an internal function

pijucha reviewed May 23, 2016
View reviewed changes

pijucha force-pushed the bug13149 branch from d4d5c93 to 6272187 Compare May 23, 2016 04:39

pijucha changed the title ~~BUG: Fix #13149 NaN to int conv. in Float64Index~~ BUG: Fix #13149 and ENH: 'copy' param in Index.astype() May 23, 2016

pijucha reviewed May 23, 2016
View reviewed changes

pijucha force-pushed the bug13149 branch from 6272187 to 8b29902 Compare May 23, 2016 18:07

jreback closed this in afde718 May 23, 2016

pijucha deleted the bug13149 branch May 24, 2016 02:16

wholmgren mentioned this pull request Oct 4, 2016

fix incompatibility with pandas 0.19 and solar position calculations pvlib/pvlib-python#247

Merged

Uh oh!

BUG: Fix #13149 and ENH: 'copy' param in Index.astype() #13209

BUG: Fix #13149 and ENH: 'copy' param in Index.astype() #13209

Uh oh!

Conversation

pijucha commented May 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jreback commented May 17, 2016

Uh oh!

pijucha commented May 17, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pijucha May 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented May 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback May 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented May 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pijucha May 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback May 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jreback commented May 23, 2016

Uh oh!

pijucha commented May 23, 2016

Uh oh!

jreback commented May 23, 2016

Uh oh!

pijucha commented May 17, 2016 •

edited

Loading

pijucha May 19, 2016 •

edited

Loading

jreback May 21, 2016 •

edited

Loading

pijucha May 23, 2016 •

edited

Loading

jreback May 23, 2016 •

edited

Loading