De-duplicate masking/fallback logic in ops #19613
Conversation
| xrav = xrav[mask] | ||
| if xrav.size: | ||
| with np.errstate(all='ignore'): | ||
| result[mask] = op(xrav, y) |
There was a problem hiding this comment.
I don't think this case is reached/reachable. If y is an ndarray then most non-trivial cases should have shape mismatches if this were ever reached.
Codecov Report
@@ Coverage Diff @@
## master #19613 +/- ##
==========================================
- Coverage 91.62% 91.6% -0.02%
==========================================
Files 150 150
Lines 48790 48795 +5
==========================================
- Hits 44703 44701 -2
- Misses 4087 4094 +7
Continue to review full report at Codecov.
|
| if isinstance(y, allowed_types): | ||
| yrav = y.ravel() | ||
| mask = notna(xrav) & notna(yrav) | ||
| result[mask] = op(np.array(list(xrav[mask])), |
There was a problem hiding this comment.
Do you know why we build the intermediate list here?
There was a problem hiding this comment.
No idea. I can try removing it and see if that affects any tests.
There was a problem hiding this comment.
Looks like a complex-dtype test would be affected.
jbrockmendel
changed the title
jreback
added
Missing-data
| # Masking NA values and fallbacks for operations numpy does not support | ||
|
|
||
| def fill_binop(left, right, fill_value): | ||
| if fill_value is not None: |
|
|
||
|
|
||
| def mask_cmp_op(x, y, op, allowed_types): | ||
| # TODO: Can we make the allowed_types arg unnecessary? |
| this, other = self.align(other, join='outer', level=level, copy=False) | ||
| new_index, new_columns = this.index, this.columns | ||
|
|
||
| def _arith_op(left, right): |
There was a problem hiding this comment.
you might be able to simplify this even more here (IOW remove _arith_op) and just in-line it
There was a problem hiding this comment.
This overlaps with #19611. This will probably get in before that does, so I'll take a look at this suggestion after rebasing there.
|
thanks! |
3 participants
Some nice deletions here.