-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TST: tests for maybe_promote (precursor to #23982) #25637
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
Changes from 10 commits
ca8f96b
a881929
e14d8a2
bc10a87
ab328c0
80d4081
4a2327c
a205ea9
79299af
1b18fde
ff3be04
9fec7f9
bf8c02d
595012c
f89e8b4
4317f77
c0b4a3f
d6001a7
f73740b
895fcb5
200365e
bb36925
a97d29b
8c982f2
a1add65
636b1f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -376,28 +376,40 @@ def unique_nulls_fixture(request): | |
FixedOffset(0), FixedOffset(-300), timezone.utc, | ||
timezone(timedelta(hours=1)), | ||
timezone(timedelta(hours=-1), name='foo')] | ||
TIMEZONE_IDS = ['None', 'UTC', 'US/Eastern', 'Asia/Tokyp', | ||
'dateutil/US/Pacific', 'dateutil/Asia/Singapore', | ||
'dateutil.tz.tzutz()', 'dateutil.tz.tzlocal()', | ||
'pytz.FixedOffset(300)', 'pytz.FixedOffset(0)', | ||
'pytz.FixedOffset(-300)', 'datetime.timezone.utc', | ||
'datetime.timezone.+1', 'datetime.timezone.-1.named'] | ||
|
||
|
||
@td.parametrize_fixture_doc(str(TIMEZONES)) | ||
@pytest.fixture(params=TIMEZONES) | ||
@td.parametrize_fixture_doc(str(TIMEZONE_IDS)) | ||
@pytest.fixture(params=TIMEZONES, ids=TIMEZONE_IDS) | ||
def tz_naive_fixture(request): | ||
""" | ||
Fixture for trying timezones including default (None): {0} | ||
""" | ||
return request.param | ||
|
||
|
||
@td.parametrize_fixture_doc(str(TIMEZONES[1:])) | ||
@pytest.fixture(params=TIMEZONES[1:]) | ||
@td.parametrize_fixture_doc(str(TIMEZONE_IDS[1:])) | ||
@pytest.fixture(params=TIMEZONES[1:], ids=TIMEZONE_IDS[1:]) | ||
def tz_aware_fixture(request): | ||
""" | ||
Fixture for trying explicit timezones: {0} | ||
""" | ||
return request.param | ||
|
||
|
||
# Generate cartesian product of tz_aware_fixture: | ||
tz_aware_fixture2 = tz_aware_fixture | ||
|
||
|
||
# ---------------------------------------------------------------- | ||
# Dtypes | ||
|
||
|
||
UNSIGNED_INT_DTYPES = ["uint8", "uint16", "uint32", "uint64"] | ||
UNSIGNED_EA_INT_DTYPES = ["UInt8", "UInt16", "UInt32", "UInt64"] | ||
SIGNED_INT_DTYPES = [int, "int8", "int16", "int32", "int64"] | ||
|
@@ -409,16 +421,16 @@ def tz_aware_fixture(request): | |
COMPLEX_DTYPES = [complex, "complex64", "complex128"] | ||
STRING_DTYPES = [str, 'str', 'U'] | ||
|
||
DATETIME_DTYPES = ['datetime64[ns]', 'M8[ns]'] | ||
TIMEDELTA_DTYPES = ['timedelta64[ns]', 'm8[ns]'] | ||
DATETIME64_DTYPES = ['datetime64[ns]', 'M8[ns]'] | ||
TIMEDELTA64_DTYPES = ['timedelta64[ns]', 'm8[ns]'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rationale for renames? Is this necessary given how massive the diff is already? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can split off the conftest-related things, of course. Regarding your specific point: The rename is really important IMO, because the However, when reading a test that is being fed by a The current state in conftest specifically contains only There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it. I'm thinking that we should separate this out. The more we can condense this PR, the better. But hold off on making that PR for the moment (just one more question re: some of other things you've changed for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can change the |
||
|
||
BOOL_DTYPES = [bool, 'bool'] | ||
BYTES_DTYPES = [bytes, 'bytes'] | ||
OBJECT_DTYPES = [object, 'object'] | ||
|
||
ALL_REAL_DTYPES = FLOAT_DTYPES + ALL_INT_DTYPES | ||
ALL_NUMPY_DTYPES = (ALL_REAL_DTYPES + COMPLEX_DTYPES + STRING_DTYPES + | ||
DATETIME_DTYPES + TIMEDELTA_DTYPES + BOOL_DTYPES + | ||
DATETIME64_DTYPES + TIMEDELTA64_DTYPES + BOOL_DTYPES + | ||
OBJECT_DTYPES + BYTES_DTYPES) | ||
|
||
|
||
|
@@ -433,6 +445,46 @@ def string_dtype(request): | |
return request.param | ||
|
||
|
||
@pytest.fixture(params=BYTES_DTYPES) | ||
def bytes_dtype(request): | ||
"""Parametrized fixture for bytes dtypes. | ||
|
||
* bytes | ||
* 'bytes' | ||
""" | ||
return request.param | ||
|
||
|
||
@pytest.fixture(params=OBJECT_DTYPES) | ||
def object_dtype(request): | ||
"""Parametrized fixture for object dtypes. | ||
|
||
* object | ||
* 'object' | ||
""" | ||
return request.param | ||
|
||
|
||
@pytest.fixture(params=DATETIME64_DTYPES) | ||
def datetime64_dtype(request): | ||
"""Parametrized fixture for datetime/timedelta dtypes. | ||
|
||
* 'datetime64[ns]' | ||
* 'M8[ns]' | ||
""" | ||
return request.param | ||
|
||
|
||
@pytest.fixture(params=TIMEDELTA64_DTYPES) | ||
def timedelta64_dtype(request): | ||
"""Parametrized fixture for datetime/timedelta dtypes. | ||
|
||
* 'timedelta64[ns]' | ||
* 'm8[ns]' | ||
""" | ||
return request.param | ||
|
||
|
||
@pytest.fixture(params=FLOAT_DTYPES) | ||
def float_dtype(request): | ||
""" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remind me again why
TIMEZONE_IDS
is needed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gfyoung
Fair point (as with the other conftest-related comments), but it was needed for me because debugging tz-tests is very tedious without knowing for which timezone the tests failed.
Related discussion: #25425 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Can you open a PR that breaks off the following changes:
TIMEZONE_IDS
DATETIME_DTYPES
renameTIMEDELTA_DTYPES
renameIt will be a small PR, but I think it's a reasonable change to be made independently of this PR and shrink it by a little bit.