-
-
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: Deprecate Series/Dataframe.to_dense/to_sparse #26684
Changes from 26 commits
a71737c
fc08e93
82c713d
39230d4
1e7c0e8
e68826c
20b8962
50d0534
933162d
dd1e6c2
c7f27fd
be14520
b12e447
2d4de51
eede9b8
0b08795
5182a1f
15909c5
104c12a
e713fb0
587b14f
1318676
9043e03
58c678a
ca14ac1
0c8f287
871ccff
a8f6c56
72aaca5
6a6e333
4e67856
4a3181b
a546a89
5fdb2f8
a627828
3d36430
9f888c5
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 |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import numpy as np | ||
import pytest | ||
|
||
VikramjeetD marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import pandas as pd | ||
from pandas.util import testing as tm | ||
|
||
|
||
@pytest.mark.filterwarnings("ignore:Sparse:FutureWarning") | ||
def test_deprecated_to_sparse(): | ||
df = pd.DataFrame({"A": [1, np.nan, 3]}) | ||
sparse_df = pd.SparseDataFrame({"A": [1, np.nan, 3]}) | ||
|
||
# Deprecated 0.25.0 | ||
with tm.assert_produces_warning(FutureWarning, | ||
check_stacklevel=False): | ||
result = df.to_sparse() | ||
tm.assert_frame_equal(result, sparse_df) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1018,13 +1018,19 @@ def test_sparse(self): | |
df = pd.DataFrame(np.random.randn(10, 4)) | ||
df.loc[:8] = np.nan | ||
|
||
sdf = df.to_sparse() | ||
# GH 26557: DEPR | ||
with tm.assert_produces_warning(FutureWarning, | ||
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. I think here, you can also a filterwarning instead (since the test already has a filterwarning for the main Sparse deprecation as well) 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. Will check on that, but we still have to come to an unanimous decision whether to keep the double deprecation warning for 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.
We should not keep the double warning at all. |
||
check_stacklevel=False): | ||
sdf = df.to_sparse() | ||
expected = df.to_json() | ||
assert expected == sdf.to_json() | ||
|
||
s = pd.Series(np.random.randn(10)) | ||
s.loc[:8] = np.nan | ||
ss = s.to_sparse() | ||
# GH 26557: DEPR | ||
with tm.assert_produces_warning(FutureWarning, | ||
check_stacklevel=False): | ||
ss = s.to_sparse() | ||
|
||
expected = s.to_json() | ||
assert expected == ss.to_json() | ||
|
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.
wont' this also trigger the SDF warnings? (should this just be changed to create a DF here?
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.
Do you mean the SDF deprecation warning? If thats the case, yes, but this was requested in the issue that this PR will address ( #26557 ).
Also, this might be helpful if someone has already hit the SDF warning through a different route, as a gentle reminder. Either way we could decide whats best and update accordingly.
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.
Yes, it already triggers the SparseDataFrame warning. But, it is still good to explicitly deprecate this method as well (in the docs, and by giving a clearer warning message, although the other one will also still be present).
I don't think we should change the behaviour, since we are deprecating it (it would also be a backwards incompatible change, as DataFrame with sparse and SparseDataFrame are not fully interchangeable).
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.
So I think we can agree to keep both the warnings and resolve this conversation?
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.
actually i would suppress the SDF
warning here as the user facing to_sparse is already good enough