Skip to content

TST/MAINT: split up test_resample.py to make it more understandable #17806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
winklerand opened this issue Oct 6, 2017 · 4 comments
Closed
Labels
Clean Resample resample method Testing pandas testing functions or related to the test suite

Comments

@winklerand
Copy link
Contributor

As discussed in #16153 (comment), test_resample.py grew sufficiently large (currently ~ 2.7 k sloc), so splitting it up should make it more understandable.

Proposal to split along the existing classes:

tests/resample (sub-dir)
|- common.py
|- test_resample_api.py
|- base.py
|- test_datetime.py
|- test_period.py
|- test_timedelta.py
|- test_resample_grouper.py
|- test_time_grouper.py

Moreover, readability of some test cases could be improved by using pytest features:

  • Convert nested for loops and "quasi-parametrized" tests (multiple test methods calling the same test logic with different parameters) to make use of @pytest.mark.parametrize decorator
  • Use pytest fixtures where appropriate
@jreback
Copy link
Contributor

jreback commented Oct 6, 2017

sgtm!

@sinhrks sinhrks added Clean Resample resample method Testing pandas testing functions or related to the test suite labels Oct 6, 2017
@jreback jreback added this to the Next Major Release milestone Oct 8, 2017
@simonjayhawkins
Copy link
Member

Any objections to opening a new PR for this?

@jreback
Copy link
Contributor

jreback commented Nov 23, 2018

go for it

@mroeschke
Copy link
Member

I think this file has been sufficiently split up. Cleanup can be a separate task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Resample resample method Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants