TST: fixtures for timezones #20365
TST: fixtures for timezones #20365
Conversation
pandas/conftest.py
Outdated
| @pytest.fixture(params=TIMEZONES) | ||
| def tz_naive(request): | ||
| """ | ||
| Fixture for trying timezones including default (None) |
There was a problem hiding this comment.
can you make this ore explicit (e.g. list the timezones here); you can programatically construct the doc string I think (to make this DRY)
There was a problem hiding this comment.
I added decorator for this
pandas/conftest.py
Outdated
| @pytest.fixture(params=TIMEZONES[1::]) | ||
| def tz_aware(request): | ||
| """ | ||
| Fixture for trying explicit timezones |
pandas/conftest.py
Outdated
|
|
||
|
|
||
| @pytest.fixture(params=TIMEZONES) | ||
| def tz_naive(request): |
There was a problem hiding this comment.
I wouldn't be averse to making the name of this slightly more explict
maybe tz_naive_fixture (maybe we should do this with the other fixtures as well, can be separate PR for that change)
| @pytest.fixture(params=[None, 'UTC', 'Asia/Tokyo', 'US/Eastern', | ||
| 'dateutil/Asia/Singapore', | ||
| 'dateutil/US/Pacific']) | ||
| def tz(request): |
| idx2 = DatetimeIndex(['2011-01-01 09:00', '2011-01-01 10:00', | ||
| '2011-01-01 11:00'], freq='H', | ||
| tz='Asia/Tokyo', name='tzidx') | ||
| @pytest.mark.parametrize('idx', |
There was a problem hiding this comment.
you can format this more like
@pytest.mark.parameterize(
'idx',
[
DatetimeIndex(......),
DatetimeIndex(....)')
e.g. to fit more on a line
Codecov Report
@@ Coverage Diff @@
## master #20365 +/- ##
==========================================
- Coverage 91.84% 91.82% -0.03%
==========================================
Files 152 152
Lines 49264 49269 +5
==========================================
- Hits 45246 45239 -7
- Misses 4018 4030 +12
Continue to review full report at Codecov.
|
|
A few thoughts, in no particular order:
|
pandas/conftest.py
Outdated
| return request.param | ||
|
|
||
|
|
||
| @pytest.fixture(params=TIMEZONES[1::]) |
There was a problem hiding this comment.
Is a single-colon OK here?
|
@jreback @jbrockmendel
Actually I had the same thought. As for this change, it affects only tests that already share similar list of timezones. I made sure that I didn't shorten or replace timezone lists used in the existing tests. Let's say test is using some timezone not covered by the fixture then list is not replaced. Several tests though got extended by one timezone (dateutil/US/Pacific) after being moved to the fixture. I thought about adding more timezones to the list but for some tests it looked a bit redundant, so I decided to save some build time.
do you mean tests like in test_timezones parametrized with tzstr? |
I think that's the right choice. This would be a decent case for using hypothesis: #17978 (but that's for another issue) |
| @@ -187,3 +187,10 @@ def decorated_func(func): | |||
| "installed->{installed}".format( | |||
| enabled=_USE_NUMEXPR, | |||
| installed=_NUMEXPR_INSTALLED)) | |||
|
|
|||
|
|
|||
| def parametrize_fixture_doc(*args): | |||
There was a problem hiding this comment.
this is nice, in a future PR can you document this itself?
There was a problem hiding this comment.
can you add docs on this here
There was a problem hiding this comment.
@jreback done
sorry didn't see this comment at first, so I did two updates (one for rebase and one for doc), I think previous builds can be cancelled
|
can you rebase this just to make sure |
|
Thanks @almaleksia! |
This is a follow up for #20287
Different tests use different lists of timezones and it's kind of hard to unify all of them. In this PR I tried to create a fixture that covers most frequent cases. Some tests use naive timezone, while others are designed to use only explicit timezones (and fail in case tz is not specified), that's why I created two fixtures to cover these two group of tests. List of timezones also includes one dateutil-timezone.
I'm not sure if described approach is correct though so I would appreciate any feedback.
git diff upstream/master -u -- "*.py" | flake8 --diff