CLN: Split test_window.py #27305
CLN: Split test_window.py #27305
Conversation
mroeschke
added
Clean
Testing
WillAyd
left a comment
WillAyd
left a comment
There was a problem hiding this comment.
Thanks - this module could definitely use a little refactor.
Not opposed to this move but thinking through if we should parametrize first instead
| self.data = self._create_dtype_data(self.dtype) | ||
| self.expects = self.get_expects() | ||
|
|
||
| def test_dtypes(self): |
There was a problem hiding this comment.
Is this the only function being tested in this module? If so I think would make sense to try and parametrize this first before moving
|
|
||
| def test_ragged_std(self): | ||
|
|
||
| df = self.ragged |
There was a problem hiding this comment.
Good case for parametrization here (and a few functions below)
pandas/tests/window/test_window.py
Outdated
| from collections import OrderedDict | ||
| from datetime import datetime, timedelta | ||
| from itertools import product | ||
| from typing import Any |
There was a problem hiding this comment.
Not opposed to this but note that we haven't done annotations in any other test module nor do we check it (mypy.ini excludes these) so would be OK to leave out for now as well
pandas/tests/window/test_window.py
Outdated
|
|
||
|
|
||
| @pytest.fixture(params=[None, 1]) | ||
| def min_periods(request) -> Any: |
There was a problem hiding this comment.
Optional[int] would be preferable as the return value here
|
@mroeschke @WillAyd let's do the straight split first, modifications later. ping on green. |
|
I think we merged something, can you merge master and ping on green. |
|
All green @jreback |
|
thanks @mroeschke |
|
it looks like this broke the ci. |
|
@simonjayhawkins can u put a link up |
|
Strange that my PRs based on earlier commit also fails. Does azure merge the PR into master before tesing? |
it looks like it. good to know. |
ghost
mentioned this pull request
4 participants
xref #19228, #26807
black pandasgit diff upstream/master -u -- "*.py" | flake8 --difftest_window.pywas getting fairly unwieldily. There a couple test classes that use a non-pytest-idomaticBaseclass, so those test classes are kept together for now. I split out the rest of the test classes into separate files as they made sense.I think the classes in
test_window.pyneed further untangling before aconftest.pyfile can be made for the new directory.Open to additional quick win optimizations.
cc @jreback