TST: further clean up of frame/test_analytics #23016
Conversation
|
Hello @h-vetinari! Thanks for submitting the PR.
|
Codecov Report
@@ Coverage Diff @@
## master #23016 +/- ##
=======================================
Coverage 92.19% 92.19%
=======================================
Files 169 169
Lines 50835 50835
=======================================
Hits 46868 46868
Misses 3967 3967
Continue to review full report at Codecov.
|
pandas/tests/frame/test_analytics.py
Outdated
| has_numeric_only=False, check_dtype=True, | ||
| check_dates=False, check_less_precise=False, | ||
| skipna_alternative=None): | ||
| def assert_stat_op_calc(opname, alternative, main_frame, has_skipna=True, |
pandas/tests/frame/test_analytics.py
Outdated
| skipna_alternative=None): | ||
| def assert_stat_op_calc(opname, alternative, main_frame, has_skipna=True, | ||
| check_dtype=True, check_dates=False, | ||
| check_less_precise=False, skipna_alternative=None): | ||
|
|
There was a problem hiding this comment.
can you add a doc-string describing things
gfyoung
added
Testing
| skipna_alternative=None): | ||
|
|
||
| f = getattr(main_frame, name) | ||
| def assert_stat_op_calc(opname, alternative, frame, has_skipna=True, |
There was a problem hiding this comment.
- Just curious, why the name change?
- In general, I believe we keep the underscore unless we intend to import this function into other test modules (which we aren't?)
There was a problem hiding this comment.
@gfyoung
It came out of review from @jreback #22733 (comment):
can this just be a module level function now? maybe rename to assert_stat_ops
I then split it into _calc and _api. Happy to add an underscore or rename it to whatever.
There was a problem hiding this comment.
Gotcha. My personal preference is underscore for the name given the reasoning I described above. The changes are good otherwise.
|
thanks |
Follow-up to #22733, where some things were left undone to ease reviewing.
Mainly:
_check_stat_op, splitting it into two functions with separate purposestest_maxetc._check_bool_optest_any_allaccording to review in TST/CLN: Fixturize frame/test_analytics #22733