-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
CLN: Deprecate pandas.SparseArray for pandas.arrays.SparseArray #30656
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.
looks good. on the tests if you can simply import SparseArray at the top rather than using pd.array.SparseArray would be better. ping on green.
@jreback now green |
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.
comment for followon, @TomAugspurger ok here?
@@ -45,7 +45,7 @@ def test_basic(self, sparse, dtype): | |||
dtype=self.effective_dtype(dtype), | |||
) | |||
if sparse: | |||
expected = expected.apply(pd.SparseArray, fill_value=0.0) | |||
expected = expected.apply(pd.arrays.SparseArray, fill_value=0.0) |
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.
ok, but i'd still prefer just using SparseArray and importing it at the top, a followup for this would be ok.
ahh turns out need to have you rebase. can you make those import changes as well. |
Yeah, good with these changes.
…On Sat, Jan 4, 2020 at 5:24 PM Jeff Reback ***@***.***> wrote:
ahh turns out need to have you rebase. can you make those import changes
as well.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30656?email_source=notifications&email_token=AAKAOIQNOEZF3ZJ63WQR32DQ4ELDNA5CNFSM4KCRC3H2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIDCO2A#issuecomment-570828648>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKAOISDBFSLG7DXQKHUGWTQ4ELDNANCNFSM4KCRC3HQ>
.
|
@Dr-Irv can u merge master |
@jreback I did the merge, but CI is failing when trying to compile |
|
thanks @Dr-Irv very nice! |
Consistency in our docs is certainly a good change, but I am not sure the deprecation of the top-level name is necessary. First, it's just historical reasons it is there (we have many things the way they are for historical reasons), and second, you can't create a sparse array as easily with a top-level function ( Anyway, besides that comment, two practical comments on this PR:
|
FutureWarning is correct here - why add this extra step / burden |
Yes, I just built an python 3.6 environment and see what you are saying, so I'll work on fixing that. |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
Change all references in code from
pd.SparseArray
topd.arrays.SparseArray
. Add deprecation message forpd.SparseArray
Per comment from @TomAugspurger here: #30644 (comment), this may require discussion.