Reduce redirection in ops #19649
Reduce redirection in ops #19649
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19649 +/- ##
==========================================
- Coverage 91.6% 91.59% -0.01%
==========================================
Files 150 150
Lines 48864 48830 -34
==========================================
- Hits 44762 44728 -34
Misses 4102 4102
Continue to review full report at Codecov.
|
gfyoung
added
Refactor
| @@ -1298,18 +1371,19 @@ def f(self, other, axis=0): | |||
| comp_method=_comp_method_PANEL, | |||
| bool_method=_arith_method_PANEL) | |||
|
|
|||
| panel_flex_funcs = dict(flex_arith_method=_flex_method_PANEL, | |||
There was a problem hiding this comment.
are you adding support for this?
There was a problem hiding this comment.
This changes zero logic, just puts a dict here instead of passing these directly in panel.
| @@ -1379,3 +1454,12 @@ def wrapper(self, other): | |||
| name = name[2:-2] | |||
| wrapper.__name__ = name | |||
| return wrapper | |||
|
|
|||
|
|
|||
| sparse_array_special_funcs = dict(arith_method=_arith_method_SPARSE_ARRAY, | |||
There was a problem hiding this comment.
This changes zero logic, just puts a dict here instead of passing these directly in sparse.array.
| @@ -1528,8 +1528,7 @@ def _extract_axis(self, data, axis=0, intersect=False): | |||
| 'minor_axis': 'columns'}) | |||
|
|
|||
| ops.add_special_arithmetic_methods(Panel, **ops.panel_special_funcs) | |||
| ops.add_flex_arithmetic_methods(Panel, ops._flex_method_PANEL, | |||
| flex_comp_method=ops._comp_method_PANEL) | |||
| ops.add_flex_arithmetic_methods(Panel, **ops.panel_flex_funcs) | |||
There was a problem hiding this comment.
why don't we name these the same for all classes
There was a problem hiding this comment.
in fact, why do we need this logic at all, cannot
ops.add_flex_arithmetic_method(cls) just do this?
or even better
ops.add_arithmetic_methods(cls)
There was a problem hiding this comment.
I like this idea. SparseSeries does some double-calls that I'll need to sort out to make this work, so let's roll this into the next pass which is Sparse-specific.
There was a problem hiding this comment.
ok this looks pretty good. I think should take this one step further (can be in another PR). where you then only need to call something like
ops.add_arithmetic_methods(cls)
in each class and push the logic of which flex/special are added to core ops
|
I think this is the only outstanding PR that os core.ops-centric. Let's try to close this out before moving forward with the remaining series/frame-implementation-de-duplication. |
|
if you can rebase and then ping on green. |
|
semi-ping. clicking through to appveyor it shows green and said it finished an hour ago; the icon hasn't updated here though. |
|
thanks. ideally would like to see a PR to simplify this further as my above comment. |
|
The appveyor build from 9355ce2 isn't actually still running is it? If so could that be clogging the appveyor pipeline? |
Among other things, this will make it easier to handle the sparse vs non-sparse Frame ops in one place.