-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
DEPR: Series ndarray properties (strides, data, base, itemsize, flags) #20721
Changes from 5 commits
c3ac291
11182b1
1aeaf2e
5339975
c78d5d6
5bd2940
160ace0
d2088d2
7687330
feb7caf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -209,6 +209,16 @@ def ceil(self, freq): | |||
class DatetimeIndexOpsMixin(object): | ||||
""" common ops mixin to support a unified interface datetimelike Index """ | ||||
|
||||
@property | ||||
def base(self): | ||||
""" return the base object if the memory of the underlying data is | ||||
shared | ||||
""" | ||||
# override deprecated property in IndexOpsMixin, as we still need | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. your explanation doesn't make sense, we are deprecating these no? you need to change the way its accessed in the code to remove the warnings. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The explanation perfectly makes sense, but you don't like the way I solved it I suppose (I agree it is not the nicest, but I was thinking that this would only be temporarily until DatetimeTZBlock is an ExtensionBlock). To fix the usage itself,
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my point is this method should not exist (as you are ddeprecateding), and would rather have you fix the usage There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I said, I agree with that. Do you have a suggestion for the second case? (inside There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, see my comment there, only check base if its an ndarray |
||||
# this for internals (DatetimeIndex/TimedeltaIndex is stored as | ||||
# values in Blocks) | ||||
return self.values.base | ||||
|
||||
def equals(self, other): | ||||
""" | ||||
Determines if two Index objects contain the same elements. | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -328,7 +328,7 @@ def test_series_agg_multi_pure_python(): | |
'F': np.random.randn(11)}) | ||
|
||
def bad(x): | ||
assert (len(x.base) > 0) | ||
assert (len(x.values.base) > 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really sure what the purpose of this assert actually is (was introduced in 71e9046) |
||
return 'foo' | ||
|
||
result = data.groupby(['A', 'B']).agg(bad) | ||
|
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.
np.asarray
slightly more idiomaticThere 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.
Ah, yes (for some reason I wanted the
copy=False
keyword, but forasarray
that is of course just the default behaviour)