-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Ensure that DatetimeArray keeps reference to original data #23956
Conversation
(cherry picked from commit 49f6495ba5ceef35a7a962f20e8b863c544592b7)
Hello @TomAugspurger! Thanks for submitting the PR.
|
@@ -99,11 +99,11 @@ def ensure_datetime64ns(arr: ndarray, copy: bool=True): | |||
|
|||
ivalues = arr.view(np.int64).ravel() | |||
|
|||
result = np.empty(shape, dtype='M8[ns]') | |||
result = np.empty(shape, dtype=NS_DTYPE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear: this change is cosmetic. The real change is below.
Codecov Report
@@ Coverage Diff @@
## master #23956 +/- ##
=======================================
Coverage 92.31% 92.31%
=======================================
Files 161 161
Lines 51471 51471
=======================================
Hits 47515 47515
Misses 3956 3956
Continue to review full report at Codecov.
|
Added tests for copy
lgtm. |
thanks! |
(cherry picked from commit 49f6495ba5ceef35a7a962f20e8b863c544592b7)
This avoids a segfault I (and possibly @jbrockmendel) were seeing on our datetimearray branches from us freeing a region of memory we didn't allocate.
I haven't really groked things yet, but
_libs/reduction.pyx::Slider
is doing some strange things with the data of the array passed to it (overwriting the buffer's.data
and.strides
attributes). One of the arguments,buff
, is a slice on the original index. It seems like ensuring that the ndarray backing the DatetimeArray sliced index is the same as the original index avoids the segfault.