CLN: refactor tests in test_moments_ewm.py #30570
Conversation
jreback
added
Testing
| @@ -263,49 +265,68 @@ class TestEwmMomentsConsistency(ConsistencyBase): | |||
| def setup_method(self, method): | |||
| self._create_data() | |||
|
|
|||
| def test_ewmcov(self): | |||
| self._check_binary_ew("cov") | |||
| def _create_binary_ew_data(self): | |||
|
|
||
| def test_ewmcov_pairwise(self): | ||
| self._check_pairwise_moment("ewm", "cov", span=10, min_periods=5) | ||
|
|
||
| def test_ewmcorr(self): | ||
| self._check_binary_ew("corr") | ||
| @pytest.mark.parametrize("min_periods", [0, 1, 2]) |
There was a problem hiding this comment.
you can just create a fixture with these min_periods (you can put in a conftest.py in this directory)
There was a problem hiding this comment.
yeah, i made a conftest.py under moments directory, and add couple fixtures there
|
|
||
| def test_ewmcorr_pairwise(self): | ||
| self._check_pairwise_moment("ewm", "corr", span=10, min_periods=5) | ||
|
|
||
| def _check_binary_ew(self, name): | ||
| def _check_binary_ew(self, name, A, B): |
There was a problem hiding this comment.
make this a free function, put in common.py
| result = func(A, B, 20, min_periods=5) | ||
| assert np.isnan(result.values[:14]).all() | ||
| assert not np.isnan(result.values[14:]).any() | ||
|
|
||
| def _check_binary_ew_min_periods(self, name, min_periods, A, B): |
There was a problem hiding this comment.
this does not need to be on the class, it can just be a free function in common.py
also rename these to something like
check_binary_ew_min_periods (de-prevatize)
There was a problem hiding this comment.
yeah, i fixed! and also combined two methods with parametrization since they are almost identical!
thanks!
pandas/tests/window/common.py
Outdated
| @@ -348,3 +348,7 @@ def get_result(obj, obj2=None): | |||
| result.index = result.index.droplevel(1) | |||
| expected = get_result(self.frame[1], self.frame[5]) | |||
| tm.assert_series_equal(result, expected, check_names=False) | |||
|
|
|||
| @staticmethod | |||
There was a problem hiding this comment.
just make this a module level function, ideally we want to get rid of all of the class based stuff eventually here.
There was a problem hiding this comment.
these also don't need to be private (leading underscore)
There was a problem hiding this comment.
just make this a module level function, ideally we want to get rid of all of the class based stuff eventually here.
ahh! I get what you meant before, sorry for misunderstanding! fixed!
| result = func(A, B, 20, min_periods=5) | ||
| assert np.isnan(result.values[:14]).all() | ||
| assert not np.isnan(result.values[14:]).any() | ||
|
|
||
| def _check_binary_ew_min_periods(self, name, min_periods, A, B): |
There was a problem hiding this comment.
this does not need to be on the class, it can just be a free function in common.py
also rename these to something like
check_binary_ew_min_periods (de-prevatize)
|
thanks @charlesdong1991 |
|
thanks, will do @jreback |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff