jorisvandenbossche commented on a change in pull request #12522:
URL: https://github.com/apache/arrow/pull/12522#discussion_r822521291
##########
File path: python/pyarrow/tests/strategies.py
##########
@@ -224,6 +233,8 @@ def _pymap(draw, key_type, value_type, size, nullable=True):
@st.composite
def arrays(draw, type, size=None, nullable=True):
+ zoneinfo = pytest.importorskip("zoneinfo")
Review comment:
I _think_ we can move this import to the `elif
pa.types.is_timestamp(ty):` block, so the test gets only skipped when using a
timestamp type
##########
File path: python/pyarrow/tests/strategies.py
##########
@@ -96,10 +99,16 @@
pa.time64('us'),
pa.time64('ns')
])
+if tzst:
+ timezones = st.one_of(st.none(), tzst.timezones())
+elif zoneinfo:
Review comment:
`zoneinfo` is not yet defined or imported at this point?
##########
File path: python/pyarrow/tests/strategies.py
##########
@@ -266,10 +277,12 @@ def arrays(draw, type, size=None, nullable=True):
min_datetime = datetime.datetime.fromtimestamp(min_int64 // 10**9)
max_datetime = datetime.datetime.fromtimestamp(max_int64 // 10**9)
try:
- offset_hours = int(ty.tz)
- tz = pytz.FixedOffset(offset_hours * 60)
+ offset = ty.tz.split(":")
+ offset_hours = int(offset[0])
+ offset_min = int(offset[1])
+ tz = datetime.timedelta(hours=offset_hours, minutes=offset_min)
Review comment:
We are not actually using this, I think? (since the `tz` that gets
generated above is either None or a zone, but never a fixed offset string?)
(we could update the `timezones` strategy to also included fixed offsets,
but since that's not supported out of the box by hypothesis, I think it is fine
to leave this as a potential follow-up)
##########
File path: python/pyarrow/tests/strategies.py
##########
@@ -266,10 +277,12 @@ def arrays(draw, type, size=None, nullable=True):
min_datetime = datetime.datetime.fromtimestamp(min_int64 // 10**9)
max_datetime = datetime.datetime.fromtimestamp(max_int64 // 10**9)
try:
- offset_hours = int(ty.tz)
- tz = pytz.FixedOffset(offset_hours * 60)
+ offset = ty.tz.split(":")
+ offset_hours = int(offset[0])
+ offset_min = int(offset[1])
+ tz = datetime.timedelta(hours=offset_hours, minutes=offset_min)
except ValueError:
- tz = pytz.timezone(ty.tz)
+ tz = zoneinfo.ZoneInfo(ty.tz)
Review comment:
This might need a `if ty.tz is not None:` ? (since the timezone strategy
can now also return None)
##########
File path: python/pyarrow/tests/test_pandas.py
##########
@@ -25,12 +25,14 @@
from datetime import date, datetime, time, timedelta, timezone
import hypothesis as h
-import hypothesis.extra.pytz as tzst
+try:
+ import hypothesis.extra.pytz as tzst
Review comment:
I _think_ we could also use the `timezones` strategy from
`pyarrow.tests.strategies` (which is already import as `past`), so then we
don't have to redefine it here with the `if tzst: .. else: ..`
##########
File path: python/pyarrow/tests/test_convert_builtin.py
##########
@@ -995,6 +994,9 @@ def test_sequence_timestamp():
'ns'
])
def test_sequence_timestamp_with_timezone(timezone, unit):
+ pytest.importorskip("pytz")
+ import pytz
Review comment:
I think you can simplify this to a single line as `pytz =
pytest.importorskip("pytz")`
(and the same for all other occurrences)
##########
File path: python/pyarrow/tests/test_types.py
##########
@@ -23,10 +23,12 @@
import pickle
import pytest
-import pytz
import hypothesis as h
import hypothesis.strategies as st
-import hypothesis.extra.pytz as tzst
+try:
+ import hypothesis.extra.pytz as tzst
Review comment:
same here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
