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.

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

BUG: Consistent division by zero behavior for Index/Series #27321

Merged
merged 29 commits into from Jul 11, 2019

Conversation

@jbrockmendel
Copy link
Member

commented Jul 10, 2019

We have two very-similar functions for masking division by zero for Index vs Series. The Index version is the more correct of the two, so I'm trying to make the Series one behave more like the Index one, eventually just using the Index one for both.

The hard part here is that IntegerArray, SparseArray, and SparseSeries have their own arithmetic implementations, and we test that they behave the same as Series. Several attempts to patch these implementations have failed (and motivated #27302, which this is rebased on top of), so for now this PR patches the tests instead of the implementations.

I think the IntegerArray implementation should be relatively easy to patch. SparseArray I'll need some help (cc @TomAugspurger) with __setitem__ and/or putmask.

@jreback jreback added this to the 0.25.0 milestone Jul 10, 2019

Show resolved Hide resolved pandas/core/sparse/frame.py Outdated
@@ -424,6 +426,11 @@ def _compare_to_dense(a, b, da, db, op):
sparse_result = op(a, b)
dense_result = op(da, db)

if op in [operator.floordiv, ops.rfloordiv] or isinstance(op, LambdaType):

This comment has been minimized.

Copy link
@jreback

jreback Jul 10, 2019

Contributor

why is this not just callable(op)?

This comment has been minimized.

Copy link
@jbrockmendel

jbrockmendel Jul 10, 2019

Author Member

Because we don't want e.g. operator.add

This comment has been minimized.

Copy link
@jreback

jreback Jul 10, 2019

Contributor

k can you add that as a comment (future PR ok), as its not obvious

This comment has been minimized.

Copy link
@jbrockmendel

jbrockmendel Jul 11, 2019

Author Member

sure

@@ -332,6 +332,9 @@ def test_ser_divmod_zero(self, dtype1, any_real_dtype):
right = pd.Series([0, 2]).astype(dtype2)

expected = left // right, left % right

This comment has been minimized.

Copy link
@jreback

jreback Jul 10, 2019

Contributor

in future PR can you give some comments on what you ar doing here (filling np.inf on the integer divide)

This comment has been minimized.

Copy link
@jbrockmendel

jbrockmendel Jul 11, 2019

Author Member

sure

@jreback

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2019

k looks fine (except for the comments which can be addressed as a followup), but we need a note for the changes in Series yes? (e.g. they are very edge casey but need a mention)

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2019

will add note. do we want to try to fix the IntegerArray and SparseArray behavior here too? IntegerArray I think I can figure out but SparseArray I've beat my head against and got nowhere.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

will add note. do we want to try to fix the IntegerArray and SparseArray behavior here too? IntegerArray I think I can figure out but SparseArray I've beat my head against and got nowhere.

no let's do as followups.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Jul 11, 2019

lgtm ping on green (note that I just merged a patch to make the numpy dev branch pass) so you may need to rebase again.

@jbrockmendel

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2019

ping

@jreback jreback merged commit 3885575 into pandas-dev:master Jul 11, 2019

14 checks passed

codecov/patch 95.65% of diff hit (target 50%)
Details
codecov/project 92.82% (target 82%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
pandas-dev.pandas Build #20190711.9 succeeded
Details
pandas-dev.pandas (Checks) Checks succeeded
Details
pandas-dev.pandas (Docs) Docs succeeded
Details
pandas-dev.pandas (Linux py35_compat) Linux py35_compat succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow) Linux py36_locale_slow succeeded
Details
pandas-dev.pandas (Linux py36_locale_slow_old_np) Linux py36_locale_slow_old_np succeeded
Details
pandas-dev.pandas (Linux py37_locale) Linux py37_locale succeeded
Details
pandas-dev.pandas (Linux py37_np_dev) Linux py37_np_dev succeeded
Details
pandas-dev.pandas (Windows py36_np15) Windows py36_np15 succeeded
Details
pandas-dev.pandas (Windows py37_np141) Windows py37_np141 succeeded
Details
pandas-dev.pandas (macOS py35_macos) macOS py35_macos succeeded
Details

@jbrockmendel jbrockmendel deleted the jbrockmendel:divzero3 branch Jul 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.