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.

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

DEPR: Series ndarray properties (strides, data, base, itemsize, flags) #20721

Merged
merged 10 commits into from
Apr 25, 2018
Merged

DEPR: Series ndarray properties (strides, data, base, itemsize, flags) #20721

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c3ac291
DEPR: Series ndarray properties (strides, data, base, itemsize, flags)
jorisvandenbossche Apr 17, 2018
11182b1
better warning message
jorisvandenbossche Apr 18, 2018
1aeaf2e
add whatsnew
jorisvandenbossche Apr 18, 2018
5339975
fix usage in tests
jorisvandenbossche Apr 18, 2018
c78d5d6
fix remaining tests
jorisvandenbossche Apr 19, 2018
5bd2940
Merge remote-tracking branch 'upstream/master' into depr-series-ndarr…
jorisvandenbossche Apr 23, 2018
160ace0
also deprecate DatetimeIndex.base
jorisvandenbossche Apr 23, 2018
d2088d2
array -> asarray
jorisvandenbossche Apr 23, 2018
7687330
fix usage of base in DatetimeIndexConstructor (_deepcopy_if_needed)
jorisvandenbossche Apr 24, 2018
feb7caf
fix usage of .base in BlockManager test_copy
jorisvandenbossche Apr 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions pandas/core/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -737,11 +737,17 @@ def item(self):
@property
def data(self):
""" return the data pointer of the underlying data """
warnings.warn("Series/Index.data is deprecated and will be "
Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to make the warning messages a bit more dynamic with something like:

"{obj}.data is deprecated...".format(obj=type(self).__name__)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, thank you (somehow I was the docstring mindset thinking we needed to adapt this upon class definition, but of course this can be done dynamically on run time :-))

"removed in a future version",
FutureWarning, stacklevel=2)
return self.values.data

@property
def itemsize(self):
""" return the size of the dtype of the item of the underlying data """
warnings.warn("Series/Index.itemsize is deprecated and will be "
"removed in a future version",
FutureWarning, stacklevel=2)
return self._ndarray_values.itemsize

@property
Expand All @@ -752,6 +758,9 @@ def nbytes(self):
@property
def strides(self):
""" return the strides of the underlying data """
warnings.warn("Series/Index.strides is deprecated and will be "
"removed in a future version",
FutureWarning, stacklevel=2)
return self._ndarray_values.strides

@property
Expand All @@ -762,13 +771,19 @@ def size(self):
@property
def flags(self):
""" return the ndarray.flags for the underlying data """
warnings.warn("Series/Index.flags is deprecated and will be "
"removed in a future version",
FutureWarning, stacklevel=2)
return self.values.flags

@property
def base(self):
""" return the base object if the memory of the underlying data is
shared
"""
warnings.warn("Series/Index.base is deprecated and will be "
"removed in a future version",
FutureWarning, stacklevel=2)
return self.values.base

@property
Expand Down
10 changes: 10 additions & 0 deletions pandas/core/indexes/datetimelike.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

@jorisvandenbossche jorisvandenbossche Apr 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The 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, .base is used in two placed:

  • Block.is_view: this I can override in DatetimeTzBlock to check self.values.values.base
  • concatenate_join_units:

    pandas/pandas/core/internals.py

    Line 5544 in ede11af

    if copy and concat_values.base is not None:
    Not fully sure about this one what to do here.

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member Author

Choose a reason for hiding this comment

The 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 concatenate_join_units) In #20745 I need to touch the same code, and checked if the values have a base attribute, but that also feels hacky

Copy link
Contributor

Choose a reason for hiding this comment

The 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.
Expand Down
18 changes: 14 additions & 4 deletions pandas/tests/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from pandas.core.base import PandasObject, NoNewAttributesMixin
from pandas.core.indexes.datetimelike import DatetimeIndexOpsMixin
from pandas._libs.tslib import iNaT
import pandas.util.testing as tm


class CheckStringMixin(object):
Expand Down Expand Up @@ -316,16 +317,25 @@ def test_ndarray_compat_properties(self):

for o in self.objs:
# Check that we work.
for p in ['shape', 'dtype', 'flags', 'T',
'strides', 'itemsize', 'nbytes']:
for p in ['shape', 'dtype', 'T', 'nbytes']:
assert getattr(o, p, None) is not None

assert hasattr(o, 'base')
# deprecated properties
for p in ['flags', 'strides', 'itemsize']:
with tm.assert_produces_warning(FutureWarning):
assert getattr(o, p, None) is not None

# not deprecated for datetime-like indices because they are used
# inside blocks
if not isinstance(o, (DatetimeIndex, TimedeltaIndex, PeriodIndex)):
with tm.assert_produces_warning(FutureWarning):
assert hasattr(o, 'base')

# If we have a datetime-like dtype then needs a view to work
# but the user is responsible for that
try:
assert o.data is not None
with tm.assert_produces_warning(FutureWarning):
assert o.data is not None
except ValueError:
pass

Expand Down