Followup Cleanup DTI test_arithmetic, ASV #19149
Conversation
| do = do | ||
| kwargs = {} | ||
|
|
||
| for n in [0, 5]: |
There was a problem hiding this comment.
I'm pretty sure that this for n in [0, 5]: down through line 563557 is intended to have one fewer level of indentation. That change is implemented up in test_dt64_with_DateOffsets.
| Timestamp('2001-06-15')]) | ||
|
|
||
| with warnings.catch_warnings(record=True): | ||
| for normalize in (True, False): |
There was a problem hiding this comment.
can parametrize on True/False as well
| for normalize in (True, False): | ||
| if isinstance(cls_name, tuple): | ||
| cls_name, kwargs = cls_name | ||
| else: |
There was a problem hiding this comment.
I don't find this very pretty maybe
try:
cls_name, kwargs = cls_name
except TypeError
kwargs = {}
There was a problem hiding this comment.
except TypeError
It actually raises a ValueError: too many values to unpack. The fact that this isn't obvious suggests we should keep the condition explicitly targeted.
The if/else can be moved outside the warnings.catch_warnings and de-indented, pretties it up a bit.
jreback
added
Testing
Codecov Report
@@ Coverage Diff @@
## master #19149 +/- ##
==========================================
- Coverage 91.53% 91.51% -0.03%
==========================================
Files 148 148
Lines 48783 48783
==========================================
- Hits 44654 44643 -11
- Misses 4129 4140 +11
Continue to review full report at Codecov.
|
| assert_func(result, exp) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize('klass,assert_func', zip([Series, DatetimeIndex], |
There was a problem hiding this comment.
i don't like the zip here, rather use a list of tuples
|
|
||
| with warnings.catch_warnings(record=True): | ||
| for n in [0, 5]: | ||
| if (cls_name in ['WeekOfMonth', 'LastWeekOfMonth', |
There was a problem hiding this comment.
comment on why you are skipping the other cases
| if (cls_name in ['WeekOfMonth', 'LastWeekOfMonth', | ||
| 'FY5253Quarter', 'FY5253'] and n == 0): | ||
| continue | ||
| offset = getattr(pd.offsets, cls_name)(n, |
There was a problem hiding this comment.
format like
offset = getattr(pd.offsets, cls_name)
n, normalize=normalize, **kwargs)
| # the offset constructor | ||
| cls_name, kwargs = cls_name | ||
| else: | ||
| cls_name = cls_name |
There was a problem hiding this comment.
remove cls_name = cls_name
| @@ -447,6 +447,110 @@ def test_dti_with_offset_series(self, tz, names): | |||
| tm.assert_series_equal(res3, expected_sub) | |||
|
|
|||
|
|
|||
| @pytest.mark.parametrize('klass,assert_func', zip([Series, DatetimeIndex], | |||
|
Added commits removing a no-longer-needed "xfail", adding a missing "assert_series_equal", and breaking up/parametrizing some giant tests. |
|
thanks! pls do the split of test_operators.py next |
Remove ASV for Timestamp.offset since that attribute has been removed.
tests.indexes.datetimes.test_arithmetic has a giant offsets test that this splits up into reasonably sized pieces.
one piece of that test currently only runs if
klass is Series, but that sub-test is now valid for DatetimeIndex too, so this PR removes that condition.parametrize tests that currently iterate over a dictionary of inputs.
A mistaken-looking indentation in that iteration ATM prevents a bunch of cases from running. This PR fixes that.
closes #xxxx
tests added / passed
passes
git diff upstream/master -u -- "*.py" | flake8 --diffwhatsnew entry