TST/REF: Fixturize constant functions in ConsistencyBase #33943
Conversation
| @@ -1477,6 +1476,7 @@ def test_rolling_consistency_std(consistency_data, window, min_periods, center): | |||
| ) | |||
|
|
|||
|
|
|||
| @pytest.mark.slow | |||
There was a problem hiding this comment.
@jbrockmendel
thanks for your comment in #33813 (comment) , indeed, i tested the execution time and seems around 40s which is quite long and should mark as slow. And the execution time of expanding_consistency_cov test is okay (~4 to 5 s)
since this change is very tiny, i think it is okay to just change it here although it is out of scope of this PR.
| @@ -18,3 +18,51 @@ def binary_ew_data(): | |||
| @pytest.fixture(params=[0, 1, 2]) | |||
| def min_periods(request): | |||
| return request.param | |||
|
|
|||
|
|
|||
| base_functions_list = [ | |||
There was a problem hiding this comment.
can you document the tuples here (e.g. what they refer to)
There was a problem hiding this comment.
yep, i added in the docs
| # GH 8269 | ||
| if no_nans: | ||
| functions = base_functions + no_nan_functions | ||
| for (f, require_min_periods, name) in functions: |
There was a problem hiding this comment.
e.g in an ideal case we don't have this loop here
There was a problem hiding this comment.
now it may be that this increases the number of tests by 10x so we don'twant to do this, but have a look see
There was a problem hiding this comment.
yeah, refer to the comment above
jreback
added
Testing
|
are tests here there any that need marking as slow? |
|
if 5s per test function is acceptable, then we are fine for tests in i marked two functions as slow in generally tests in rolling takes longer and for the rest of tests in rolling, basically all <15s, and in total ~250s, so maybe it's okay to leave them not as slow? @jreback |
|
I will double check a bit, but would be nice to get the ballpark for the threshold for slow? Update: other relatively big ones: i also mark them as slow |
|
emm, the error does not seem related. |
|
gentle ping @jreback |
|
thanks @charlesdong1991 very nice! |
This PR fixturizes the last constants (
base_functionsandno_nan_functions) in ConsistencyBase, and take corresponding tests out of class.After this one, only constants in
Baseclass needs to be fixed (ifBaseis also wanted to be cleaned)cc @jreback