BUG: pytz.AmbiguousTimeError not caught in hypothesis test test_on_offset_implementations (GH41906)

[jackzyliu]

• closes BUG: pytz.AmbiguousTimeError not caught in hypothesis test_on_offset_implementations  #41906
• tests added / passed
• Ensure all linting tests pass, see here for how to run them
• whatsnew entry: let me know if needed

[mroeschke]

One comment otherwise LGTM

[jreback]

is it possible to add the offending test case?

[jreback]

is it possible add the offending test case as well?

[jackzyliu]

is it possible to add the offending test case?

Sure, any preference between 1) a separate test with assertRaises and 2) just an @example(dt, offset) to make sure the case is always tested?

[jreback]

prefer 1

[mroeschke]

Could you make these parameterized arguments instead? i.e.

def test_nontick_offset_with_ambiguous_time_error(original_dt, target_dt, offset, tz):

[jackzyliu]

no problem

[jackzyliu]

@jreback @mroeschke

I took at a quick look at the failing Africa/Kinshasa test case (also the original offending test case) under actions-37-minimum_versions.yaml. As expected, pytz=2017.3 does not yet contain the DST information for Africa/Kinshasa that triggers AmbiguoutTimeError - namely datetime.datetime(1905, 6, 30, 23, 46, 25),.

python=3.8 pytz=2020.1:

In[6]: import pytz
In[7]: pytz.__version__
Out[7]: '2021.1'
In[8]: pytz.timezone('Africa/Kinshasa')._utc_transition_times
Out[8]: 
[datetime.datetime(1, 1, 1, 0, 0),
 datetime.datetime(1905, 6, 30, 23, 46, 25),      # ----> triggering transition time
 datetime.datetime(1908, 7, 1, 0, 0),
 datetime.datetime(1913, 12, 31, 23, 46, 25),
 datetime.datetime(1919, 8, 31, 23, 30)]

python=3.7 pytz=2017.3:

>>> import pytz
>>> pytz.__version__
'2017.3'
>>> pytz.timezone('Africa/Kinshasa')._utc_transition_times
[datetime.datetime(1, 1, 1, 0, 0), datetime.datetime(1919, 8, 31, 23, 46, 24)]

Are you okay with removing this original offending test case in test_nontick_offset_with_ambiguous_time_error?

[jreback]

@jackzyliu yeah go ahead and skip for py37 (going to merge this to 1.4 which is going to have 3.7 removed anyhow), so just xfail for rn.

[jackzyliu]

@jreback Got it! Just updated it accordingly and rebased after master.

[jreback]

thanks @jackzyliu