Skip to content

Closes #8943: COMPAT: periods needs coercion to np.int64 #9078

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
Merged
merged 1 commit into from
Dec 17, 2014
Merged

Closes #8943: COMPAT: periods needs coercion to np.int64 #9078

merged 1 commit into from
Dec 17, 2014

Conversation

@Garrett-R
Copy link
Contributor

@Garrett-R Garrett-R commented Dec 15, 2014

Let me know if this needs any work.

closes #8943
@jreback jreback added the Compat pandas objects compatability with Numpy or Python functions label Dec 16, 2014
@jreback jreback added this to the 0.16.0 milestone Dec 16, 2014
@jreback
Copy link
Contributor

jreback commented Dec 16, 2014

does the test actually trigger the error?

pls add a release note (bug fixes in v0.16.0) as well
@Garrett-R Garrett-R force-pushed the fix_8943 branch 4 times, most recently from 100701e to 7d2dc6b Compare December 16, 2014 22:17
@Garrett-R
Copy link
Contributor Author

Garrett-R commented Dec 16, 2014

Yes it does. In fact, it was overkill, so I simplified the test and set periods to just 10. I added some comments inside the test to explain what I'm testing for and why it overflows.

I tested it on 32-bit Linux (in VirtualBox) without the type conversions and it fails as expected.

====================================================================== FAIL: test_time_overflow_for_32bit_machines (pandas.tests.test_index.TestDatetimeIndex) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/garrettvb/Desktop/pandas/pandas/tests/test_index.py", line 1921, in test_time_overflow_for_32bit_machines self.assertEqual(len(idx1), periods) AssertionError: 2 != 10
@Garrett-R
Copy link
Contributor Author

Garrett-R commented Dec 16, 2014

Also, I added "(bug fixes in v0.16.0)" to the commit message. Is that what you meant by "add a release note"?
@shoyer
Copy link
Member

shoyer commented Dec 17, 2014

@Garrett-R see here for release notes: https://github.com/pydata/pandas/blob/master/doc/source/whatsnew/v0.16.0.txt

take a look at one of the older ones for examples.
@Garrett-R
Copy link
Contributor Author

Garrett-R commented Dec 17, 2014

Oh, haha, thanks! I'll take a look.
@Garrett-R Garrett-R force-pushed the fix_8943 branch 2 times, most recently from 83008b9 to 7b127d9 Compare December 17, 2014 10:38
shoyer
shoyer reviewed Dec 17, 2014
View reviewed changes
doc/source/whatsnew/v0.16.0.txt Outdated
Copy link
Member

@shoyer shoyer Dec 17, 2014

Choose a reason for hiding this comment

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

spelling: "defatuls"
@Garrett-R
Closes pandas-dev#8943 COMPAT: periods needs coercion to np.int64
c1a4fbd 
In _generate_regular_range, we explicitly cast `periods` to np.int64 to prevent overflow in the function tseries.index._generate_regular_range. Note: we don't bother casting it for the function call generate_range since there's no danger of overflow in that function.
@Garrett-R
Copy link
Contributor Author

Garrett-R commented Dec 17, 2014

Sorry about the typo. OK, hopefully everything's in order now.
shoyer added a commit that referenced this pull request Dec 17, 2014
@shoyer
Merge pull request #9078 from Garrett-R/fix_8943
890734d 
Closes #8943: COMPAT: periods needs coercion to np.int64
@shoyer shoyer merged commit 890734d into pandas-dev:master Dec 17, 2014
@shoyer
Copy link
Member

shoyer commented Dec 17, 2014

Thanks @Garrett-R !
@Garrett-R Garrett-R deleted the fix_8943 branch December 18, 2014 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Compat pandas objects compatability with Numpy or Python functions

3 participants

@Garrett-R @jreback @shoyer