Skip to content

Tests for Deprecate SparseArray for python 3.6 and 3.7 and fixes to other deprecation tests #30799

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

Merged
jreback merged 22 commits into from
Jan 9, 2020
Merged

Tests for Deprecate SparseArray for python 3.6 and 3.7 and fixes to other deprecation tests #30799

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4ace5eb
fix deprecation test for python 3.7
Dr-Irv Jan 7, 2020
76dbae4
sparsearray OK for 3.6
Dr-Irv Jan 7, 2020
2bbc831
handle 3.6/3.7 diffs
Dr-Irv Jan 7, 2020
def8d28
formatted for black
Dr-Irv Jan 7, 2020
a32d4dc
Merge remote-tracking branch 'upstream/master' into sparsedepr
Dr-Irv Jan 7, 2020
ea7b959
split test_api for 3.6 vs 3.7
Dr-Irv Jan 7, 2020
385f6d3
Merge branch 'sparsedepr' of https://github.com/Dr-Irv/pandas into sp…
Dr-Irv Jan 7, 2020
f5d0357
Merge branch 'sparsedepr' of https://github.com/Dr-Irv/pandas into sp…
Dr-Irv Jan 7, 2020
2e0bab0
datetime fixes
Dr-Irv Jan 7, 2020
52a7d03
Merge branch 'sparsedepr' of https://github.com/Dr-Irv/pandas into sp…
Dr-Irv Jan 7, 2020
5c59f84
make datetime work with 3.6
Dr-Irv Jan 8, 2020
461b0fc
Merge remote-tracking branch 'upstream/master' into sparsedepr
Dr-Irv Jan 8, 2020
f1b89f4
use instancecheck. Modify SparseArray for 3.6 to use metaclass pattern
Dr-Irv Jan 8, 2020
f08ffc9
Merge branch 'sparsedepr' of https://github.com/Dr-Irv/pandas into sp…
Dr-Irv Jan 8, 2020
aa73611
Merge remote-tracking branch 'upstream/master' into sparsedepr
Dr-Irv Jan 8, 2020
4b7e4bf
Merge branch 'sparsedepr' of https://github.com/Dr-Irv/pandas into sp…
Dr-Irv Jan 8, 2020
c31afa1
fix mypy issue for methods with no arguments
Dr-Irv Jan 8, 2020
26dc90a
Merge remote-tracking branch 'upstream/master' into sparsedepr
Dr-Irv Jan 9, 2020
7861c98
snake_case and better returns on isinstance
Dr-Irv Jan 9, 2020
7947a81
shorten up imports for datetime and SparseArray
Dr-Irv Jan 9, 2020
d78ecea
revert import simplification to make mypy happy
Dr-Irv Jan 9, 2020
7cce2c7
Merge remote-tracking branch 'upstream/master' into sparsedepr
Dr-Irv Jan 9, 2020
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
Prev Previous commit
Next Next commit
sparsearray OK for 3.6
  • Loading branch information
@Dr-Irv
Dr-Irv committed Jan 7, 2020
commit 76dbae4e9d5fb4a38d63f471126dece15ca1baf8
30 changes: 28 additions & 2 deletions pandas/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,35 @@ def __getattr__(self, item):

datetime = __Datetime().datetime

class SparseArray:
pass
class __SparseArray(pandas.core.arrays.sparse.SparseArray):
Copy link
Member

@jorisvandenbossche jorisvandenbossche Jan 8, 2020

Choose a reason for hiding this comment

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

I am not sure we can do it this way for SparseArray, as that breaks isinstance checks

Copy link
Contributor

@TomAugspurger TomAugspurger Jan 8, 2020

Choose a reason for hiding this comment

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

To confirm, you mean something like

isinstance(pd.array([1, 2, 3], dtype="Sparse"), pd.SparseArray)

will be false? Since it's an instance of the parent pd.arrays.SparseArray, not the child pd.SparseArray?

Copy link
Member

@jorisvandenbossche jorisvandenbossche Jan 8, 2020

Choose a reason for hiding this comment

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

Yes.

(in general, deprecating moving classes is annoying)

For datetime and np the same is true of course, but I think it is much less likely that someone will do that.

Copy link
Contributor Author

@Dr-Irv Dr-Irv Jan 8, 2020

Choose a reason for hiding this comment

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

This is again another issue with python 3.6 handling the deprecation of a module attribute. That's why they implemented PEP 562 (https://www.python.org/dev/peps/pep-0562/) for python 3.7. Making a reference using python 3.6 to pd.SparseArray (without a constructor) produce a warning would be quite involved (see the Rationale in the PEP discussion), and I don't think it is worth introducing that level of complexity to pandas to handle deprecation messages of a class for users of an older version of python.

It's why I suggested above to merge in this pull request, and we can leave this as an open issue.

Copy link
Member

@jorisvandenbossche jorisvandenbossche Jan 8, 2020

Choose a reason for hiding this comment

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

I know it's an issue with python 3.6, but that we still support 3.6 now is a reality we have to live with.
So my suggestion would be to simply don't try to raise a warning in python 3.6, and just import the existing class in that case (by the time we are going to enforce this deprecation, we maybe won't support py3.6 anymore, so then that is less of an issue). Or simply don't deprecate pd.SparseArray at all.

Copy link
Contributor

@TomAugspurger TomAugspurger Jan 8, 2020

Choose a reason for hiding this comment

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

Or simply don't deprecate pd.SparseArray at all.

That's probably my preference until we support just 3.7+.

Though @Dr-Irv this PR is also fixing other issues with the other deprecations on master? Taking a closer look now.

Copy link
Contributor Author

@Dr-Irv Dr-Irv Jan 8, 2020

Choose a reason for hiding this comment

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

@TomAugspurger Yes, there were other issues with deprecations on master that I addressed, especially in the testing code.


def __warnSparseArray(self):
import warnings
warnings.warn(
"The pandas.SparseArray class is deprecated "
"and will be removed from pandas in a future version. "
"Use pandas.arrays.SparseArray instead.",
FutureWarning,
stacklevel=3,
)

def __init__(
self,
data,
sparse_index=None,
index=None,
fill_value=None,
kind="integer",
dtype=None,
copy=False,
):
self.__warnSparseArray()
super().__init__(data, sparse_index, index, fill_value, kind, dtype, copy)

def __getattr__(self, name):
return super().__getattribute__(name)

SparseArray = __SparseArray

# module level doc-string
__doc__ = """
Expand Down
10 changes: 9 additions & 1 deletion pandas/tests/api/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class TestPDApi(Base):
"NamedAgg",
]
if not compat.PY37:
classes.extend(["Panel", "SparseSeries", "SparseDataFrame", "SparseArray"])
classes.extend(["Panel", "SparseSeries", "SparseDataFrame"])
deprecated_modules.extend(["np", "datetime"])

# these are already deprecated; awaiting removal
Expand Down Expand Up @@ -206,12 +206,17 @@ def test_api(self):
self.lib
+ self.misc
+ self.modules
+ self.deprecated_modules
+ self.classes
+ self.deprecated_classes
+ self.deprecated_classes_in_future
+ self.funcs
+ self.funcs_option
+ self.funcs_read
+ self.funcs_json
+ self.funcs_to
+ self.deprecated_funcs_in_future
+ self.deprecated_funcs
+ self.private_modules,
self.ignored,
)
Expand All @@ -231,6 +236,9 @@ def test_depr(self):
elif depr == "datetime":
deprecated = getattr(pd, "__Datetime")
deprecated().__getattr__(dir(pd.datetime)[-1])
elif depr == "SparseArray":
deprecated = getattr(pd, depr)
deprecated([])
else:
deprecated = getattr(pd, depr)
deprecated.__getattr__(dir(deprecated)[-1])
Expand Down