REF: separate bloated test #28081
REF: separate bloated test #28081
Conversation
| with pytest.raises(TypeError): | ||
| s_0123 & np.NaN | ||
| with pytest.raises(TypeError): | ||
| s_0123 & 3.14 | ||
| with pytest.raises(TypeError): | ||
| s_0123 & [0.1, 4, 3.14, 2] | ||
| with pytest.raises(TypeError): | ||
| s_0123 & np.array([0.1, 4, 3.14, 2]) |
There was a problem hiding this comment.
this assertion is the only new one, everything else is just moved around
gfyoung
added
Refactor
Separate PR(s). Lots of small steps. |
| @@ -791,28 +791,33 @@ def wrapper(self, other): | |||
| self, other = _align_method_SERIES(self, other, align_asobject=True) | |||
| res_name = get_op_result_name(self, other) | |||
|
|
|||
| # TODO: shouldn't we be applying finalize whenever | |||
| # not isinstance(other, ABCSeries)? | |||
| finalizer = ( | |||
There was a problem hiding this comment.
What are the code changes here required for? Separate from the actual test split?
There was a problem hiding this comment.
The goal is to isolate Series-specific parts of the function from EA/ndarray parts (see also _arith_method_SERIES and _comp_method_SERIES in #28066).
There was a problem hiding this comment.
Is there a way to move the tests without making code changes? I'm admittedly not very well versed with this code so I might be missing something simple, but I think test and code changes as separate PRs is a safer approach
There was a problem hiding this comment.
That's basically the idea of this PR. This is technically a code change, but it is about as benign as they come
|
thanks @jbrockmendel |
Working on logical ops I'm finding a bunch of internal inconsistencies that will need to be addressed. Since those are liable to have large diffs, this starts off with refactor that leaves behavior unchanged: simplify the method itself, and split a giant test into well-scoped ones.