BUG/WIP: segfault manifesting with dateutil=2.6 w.r.t. replace when timezones are present
State: closed

closes #14621

This is very close, but a stubborn fall transition issue. any ideas? (note that this does not solve #7825 but should be a simple change after I think)
cc @ischwabacher 
cc @rockg
cc @sinhrks
cc @adamgreenhall
cc @pganssle  

```
(pandas) [Thu Nov 10 09:14:13 ~/pandas]$ nosetests  pandas/tseries/tests/test_offsets.py pandas/tseries/tests/test_timezones.py  -s 
...................S..S...........................................................................................................................................................................F...........................S..............................................................S...S.......................F................................................................................................................................................
======================================================================
FAIL: test_fallback_plural (pandas.tseries.tests.test_offsets.TestDST)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jreback/pandas/pandas/tseries/tests/test_offsets.py", line 4900, in test_fallback_plural
    expected_utc_offset=hrs_post)
  File "/Users/jreback/pandas/pandas/tseries/tests/test_offsets.py", line 4850, in _test_all_offsets
    self._test_offset(offset_name=name, offset_n=n, **kwds)
  File "/Users/jreback/pandas/pandas/tseries/tests/test_offsets.py", line 4856, in _test_offset
    self.assertTrue(get_utc_offset_hours(t) == expected_utc_offset)
AssertionError: False is not true

======================================================================
FAIL: test_ambiguous_flags (pandas.tseries.tests.test_timezones.TestTimeZoneSupportDateutil)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/jreback/pandas/pandas/tseries/tests/test_timezones.py", line 524, in test_ambiguous_flags
    freq="H"))
AssertionError: Times[28 chars]00', tz='dateutil//usr/share/zoneinfo/Europe/London', freq='H') != Times[28 chars]00', tz='dateutil//usr/share/zoneinfo/Europe/London', freq='H')

----------------------------------------------------------------------
Ran 458 tests in 5.788s

FAILED (SKIP=5, failures=2)
```

Labels:
name: Bug
name: Timezones
description: Timezone data dtype
name: Compat
description: pandas objects compatability with Numpy or Python functions

Comments:
I'm not sure exactly how or if you have implemented ambiguous time support in `dateutil`, but it may be that some of your assumptions are now flawed, since we implemented proper ambiguous time support in version 2.6.0.  The biggest change (and this is coming from PEP-495, _not_ something that I made up, I assure you) is that by default unspecified ambiguity now defaults to the _first_ time, not the Standard side.

The plus side is that as long as all arithmetic is done in a fixed-offset zone like UTC, `datetime.astimezone(dateutil_tzinfo)` will just work for all times, and if you implement a `fold` attribute in `Timestamp` objects to reflect the forthcoming change in `datetime` objects in Python 3.6, then `replace` will also just work (else there would be no way to specify whether or not you are on the DST side).

I will see if I can find where you are implementing dateutil support, but you may be faster at that side of it.

Oh, actually, I didn't realize you were pinning dateutil to 2.5.3 for some reason. I don't think that's a good idea. It's almost certainly better to just fix `replace`...

@pganssle we r pinning various versions of dateutil for testing
but several also install latest 

I see, I didn't realize that.

Looking into this code a bit, I'm not really surprised that it's fragile with respect to dateutil changes. Given the lack of previous ambiguous time support it's understandable, but there's a lot of hacks in here related to `dateutil` that rely on undocumented private features of `tzfile` objects.

I think I can make things much easier by introducing two new features in the next release - an official API for retrieving transitions from `tzfile` objects and an official API for retrieving the canonical name of a given zone. Then you can presumably make better use of the official APIs and reduce the fragility of your code. Please let me know if there are other places that you are relying on undocumented features so we can figure out how to expose something stable.

@pganssle Can you move that last comment to a new issue to discuss this? As I don't think it is related to the actual code changes in this PR? (for reviewing this PR)

i changed the pin on the osx build arbitrarily (could have been 3.4) 
the 3.5 is always latest and otherwise have various pins

@pganssle certainly welcome improved apis!

we can easily just use them if available (based on version or duck typing)
want to remain back compat (though we should prob drop <2 dateutil support, maybe even make more recent required)

@pganssle is there is a list of releases and dates when they were released?

csn u show what changed for ambiguous time support in 2.5.3 -> 2.6
wrt transitions -  i think we need to change our code a bit (if u can show the current api would be great)

@jorisvandenbossche I can move it, yes, I'll make something a bit more formal later. I was just commenting on this because the use of undocumented features is likely a good place to look for possible changes. I think in this case, though, the issue is that PEP-495 actually changes the default fallback for unaware datetimes:

See:

``` python
from dateutil import tz
from datetime import datetime, timedelta

LON = tz.gettz('Europe/London')
dt1 = datetime(2013, 10, 26, 23, tzinfo=LON)
dt2 = datetime(2013, 10, 27, 1, tzinfo=LON)
dt3 = tz.enfold(dt2, fold=1)

print(dt1)   # 2013-10-26 23:00:00+01:00
print(dt2)   # 2013-10-27 01:00:00+01:00
print(dt3)   # 2013-10-27 01:00:00+00:00
```

I was expecting that no one was relying on this behavior because there was originally no actual way to construct `datetime` values with DST during ambiguous times, and I was adding this feature. Had I been involved in the PEP-495 discussions early enough, I would definitely have advocated using `fold=1` as "DST", not "STD", but it is what it is.

If you have the datetime as UTC already, it's very easy, just use `.astimezone()` and you will have a properly localized date (this is the case for pytz as well, actually). Otherwise, you can use `tz.datetime_ambiguous(dt, tz=None)` to check if a datetime is ambiguous and decide which side of the fold you want to be on using `tz.enfold()` (`fold` is ignored for non-ambiguous dates, so you can just enfold everything if you prefer). You can use `tz.datetime_exists(dt, tz=None)` to see if a date is in a "gap" in the given zone.

Evidently, I forgot to add these functions to `__all__`, on `tz`, so they haven't shown up in the documentation. A list of releases with changes is [here](https://github.com/dateutil/dateutil/releases).

```
In [6]: dateutil.__version__
Out[6]: '2.5.3'

In [19]: from dateutil.tz import tzfile

In [20]: dt = datetime.datetime(2013, 11, 3, 1, 59, 59, 999999, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))

In [21]: dt
Out[21]: datetime.datetime(2013, 11, 3, 1, 59, 59, 999999, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))

In [22]: dateutil.__version__
Out[22]: '2.5.3'

In [23]: dt.dst()
Out[23]: datetime.timedelta(0)

In [24]: dt.tzname()
Out[24]: 'PST'

In [25]: dt + datetime.timedelta(0, 180)
Out[25]: datetime.datetime(2013, 11, 3, 2, 2, 59, 999999, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))

In [26]: (dt + datetime.timedelta(0, 180)).dst()
Out[26]: datetime.timedelta(0)

In [27]: (dt + datetime.timedelta(0, 180)).tzname()
Out[27]: 'PST'
In [7]: t = pd.Timestamp('2013-11-03 01:59:59.999999').tz_localize('dateutil//usr/share/zoneinfo/US/Pacific', ambiguous=0)

In [8]: t
Out[8]: Timestamp('2013-11-03 01:59:59.999999-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

In [9]: t + datetime.timedelta(0, 180)
Out[9]: Timestamp('2013-11-03 02:02:59.999999-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

# using pandas
In [10]: t = pd.Timestamp('2013-11-03 01:59:59.999999').tz_localize('dateutil//usr/share/zoneinfo/US/Pacific', ambiguous=1)

In [11]: t
Out[11]: Timestamp('2013-11-03 01:59:59.999999-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

In [12]: t + datetime.timedelta(0, 180)
Out[12]: Timestamp('2013-11-03 01:02:59.999999-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

```

2.6.0

```
In [21]: dateutil.__version__
Out[21]: '2.6.0'

In [32]:  dt = datetime.datetime(2013, 11, 3, 1, 59, 59, 999999, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))
    ...: 

In [33]: dt
Out[33]: datetime.datetime(2013, 11, 3, 1, 59, 59, 999999, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))

In [34]: dt.dst()
Out[34]: datetime.timedelta(0, 3600)

In [35]: dt.tzname()
Out[35]: 'PDT'

In [36]: dt + datetime.timedelta(0, 180)
    ...: 
Out[36]: datetime.datetime(2013, 11, 3, 2, 2, 59, 999999, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))

In [37]: (dt + datetime.timedelta(0, 180)).dst()
    ...: 
Out[37]: datetime.timedelta(0)

In [38]: (dt + datetime.timedelta(0, 180)).tzname()
    ...: 
Out[38]: 'PST'

# pandas
In [39]: t = pd.Timestamp('2013-11-03 01:59:59.999999').tz_localize('dateutil//usr/share/zoneinfo/US/Pacific', ambiguous=0)

In [40]: t
Out[40]: Timestamp('2013-11-03 01:59:59.999999-0700', tz='dateutil//usr/share/zoneinfo/US/Pacific')

In [41]: t + datetime.timedelta(0, 180)
Out[41]: Timestamp('2013-11-03 02:02:59.999999-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

In [42]: t = pd.Timestamp('2013-11-03 01:59:59.999999').tz_localize('dateutil//usr/share/zoneinfo/US/Pacific', ambiguous=1)

In [43]: t
Out[43]: Timestamp('2013-11-03 01:59:59.999999-0700', tz='dateutil//usr/share/zoneinfo/US/Pacific')

In [44]: t + datetime.timedelta(0, 180)
Out[44]: Timestamp('2013-11-03 01:02:59.999999-0700', tz='dateutil//usr/share/zoneinfo/US/Pacific')

```

So now I am all confused, I _think_ that 2013/11/03 02:02:59.... is an invalid time, so I _think_ that [44] and [12] are right. I _think_ that we are testing the wrong behavior here.

any thoughts welcome (the 2.5.3 and 2.6.0) are in different ipython sessions, so output numbers might be confusing.

[44] doesn't seem right to me as the time switched over so it should be -0800 rather than -0700.  And also [41] has the right offset but it should be 01:02.  Similar with [12] that should go to 02:02 since it has already switched back as it is -0800 like [9] does. 

Also [10]/[11] above is wrong, that should be -0700 since you are specifying that it is DST.

Also, 2013/11/03 02:02:59 is not an invalid time since it is outside the DST change window which is from [1:00, 2:00).

[44] should look like this

```
In [17]: t
Out[17]: Timestamp('2013-11-03 01:59:59.999999-0700', tz='US/Pacific')

In [18]: t + timedelta(0, 180)
Out[18]: Timestamp('2013-11-03 01:02:59.999999-0800', tz='US/Pacific')
```

@rockg @jreback My understanding is that `timedelta()` is to be considered something that moves the "hands" of the "wall clock", and there is no way to use `timedelta()` on `datetime`s with non-fixed offset zones in such a way that it cannot produce ambiguous or non-existent dates. Consider this case:

``` python
>>> from dateutil import tz
>>> from datetime import datetime, timedelta
>>> PT = tz.gettz('US/Pacific')
>>> dt = datetime(2013, 11, 2, 12, tzinfo=PT)
>>> dt + timedelta(days=1)
datetime.datetime(2013, 11, 3, 12, 0, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))
```

If you think of `timedelta` as adding 24 hours to the absolute time, then this would return the counter-intuitive result of `2013-11-03 11:00:00`, because there are actually 25 hours on November 3rd. I made an effort at providing an `absolutedelta` class that does this in dateutil/dateutil#281, but when I was implementing it (and after some caution in dateutil/dateutil#228), it became clear to me that this approach is a bit unnecessary in most cases and more complicated than is warranted in others.

If you want to override the default python semantics and have `timedelta` give you an "absolute" difference in times, you should just perform all arithmetical operations in a fixed-offset zone, so:

``` python
>>> (dt.astimezone(tz.tzutc()) + timedelta(days=1)).astimezone(PT)
datetime.datetime(2013, 11, 3, 11, 0, tzinfo=tzfile('/usr/share/zoneinfo/US/Pacific'))
>>> dt2 = datetime(2013, 11, 3, 0, 30, tzinfo=PT)
>>> str((dt2.astimezone(tz.tzutc()) + timedelta(hours=1)).astimezone(PT))
'2013-11-03 01:30:00-07:00'
>>> str((dt2.astimezone(tz.tzutc()) + timedelta(hours=2)).astimezone(PT))
'2013-11-03 01:30:00-08:00'
```

I don't really see why `2013-11-03 02:03` is an invalid time. That's an unambiguous time in PST.

@pganssle pandas is just trying NOT to change behavior. if we want something to change that's another issue. this PR is just trying to reconcile the differences in dateutil 2.5.3 and 2.6 and to have it work on either version correctly.

@rockg 10/11 is from existing master and 2.5.3

so I think that we are then testing with an incorrect expectation? 

@jreback If you want the old default behavior from dateutil < 2.6.0, then you should do this:

``` python
from dateutil import tz
datetime_ambiguous = getattr(tz, 'datetime_ambiguous', lambda dt, tz=None: return False)
enfold = getattr(tz, 'enfold', lambda x, fold=1: return x)

def add_timedelta(dt, td):
    dt_out = dt + td
    if datetime_ambiguous(dt_out):
        return enfold(dt_out, fold=1)
    else:
        return dt_out
```

This does not handle `fold` correctly, mind you. I was sort of under the impression that you had your own semantics for what `timedelta` does, because your tests seem to indicate that `Timestamp` objects actually already do enough hacking around `dateutil`'s lack of support for ambiguous times that it works as expected. In that case, you're probably best off automatically detecting whether it's a pre-PEP495 `dateutil` or a post-PEP495 `dateutil` and use a code path that uses the hacks or uses the proper API as appropriate, and eventually when the dateutil dependency is pinned to `>= 2.6.0`, drop the hacked-up code.

As for [10]/[11], I'm not sure what your implementation is, but if you're using `dateutil`'s built-in methods, there's actually no way to produce a datetime that is on the DST side of an ambiguous date. I wouldn't be surprised if that's why the 2.5.3 section is doing the wrong thing.

Yes, the test is wrong.  This is how it looks with pytz and is correct.  I don't believe we are calling any specific dateutil methods but merely getting transition times, etc. from the timezone definition.

```
In [9]: t = pd.Timestamp('2013-11-03 01:59:59.999999').tz_localize('US/Pacific', ambiguous=1)

In [10]: t
Out[10]: Timestamp('2013-11-03 01:59:59.999999-0700', tz='US/Pacific')
```

@rockg I could be wrong, but you must be using dateutil methods somewhere, otherwise the behavior wouldn't have changed between versions 2.5.3 and 2.6.0, right?  Is it possible that you use the transitions taken from the `tzfile` and use them to calculate the correct time in UTC, then call `dt_in_utc.astimezone(the_dateutil_tz)`? If so, you would still expect the observed behavior. Even if you are storing it as UTC and call `astimezone()` every time for the purposes of calling `.tzname` and `dst`, you'll still see this behavior because it was not possible to generate a `datetime` with a `dateutil` time zone attached that resolves to the DST side.

This is fixed in `dateutil` 2.6.0 and you can consider the behavioral change a bugfix.

so .replace is called in the repr this may be affecting things 

Looks like the underlying representation is correct, but when you try and use `dst()`, `utcoffset()`, `tzname()` or the other parts of the interface that call `tzinfo` methods, it resolves incorrectly (likely because it's calling dateutil methods to do this):

``` python
>>> dateutil.__version__
'2.5.3'

>>> t_std = pd.Timestamp('2013-11-03 01:59:59.9999').tz_localize('dateutil/US/Pacific',ambiguous=0)
>>> t_std
Timestamp('2013-11-03 01:59:59.999900-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

>>> t_dst = pd.Timestamp('2013-11-03 01:59:59.9999').tz_localize('dateutil/US/Pacific', ambiguous=1)
>>> t_dst
Timestamp('2013-11-03 01:59:59.999900-0800', tz='dateutil//usr/share/zoneinfo/US/Pacific')

>>> t_std.astimezone('UTC')
Timestamp('2013-11-03 09:59:59.999900+0000', tz='UTC')

>>> t_dst.astimezone('UTC')
Timestamp('2013-11-03 08:59:59.999900+0000', tz='UTC')

>>> t_std.dst()
datetime.timedelta(0)

>>> t_dst.dst()
datetime.timedelta(0)
```

You may be able to jump through hoops to fix this, but I don't think it would be out of bounds to just say it's an upstream bug in dateutil and recommend upgrading to the latest version.

Ok I have a consistent test for the difference now, working for 2.5.3 with this PR. (these compare pytz and dateutil) I think there exists a bug in [5]/[6](the UTC times are same, just the ambiguous tz looks wrong for dateutil). ok. commit is https://github.com/pandas-dev/pandas/pull/14631/commits/cee7edccc2210b87db7da08085d2f25a6b7c2799

```
In [2]: dateutil.__version__
Out[2]: '2.5.3'

In [3]: Timestamp('2013-10-27 01:00:00').tz_localize('Europe/London', ambiguous=0)
Out[3]: Timestamp('2013-10-27 01:00:00+0000', tz='Europe/London')

In [4]: Timestamp('2013-10-27 01:00:00').tz_localize('dateutil/Europe/London', ambiguous=0)
Out[4]: Timestamp('2013-10-27 01:00:00+0000', tz='dateutil//usr/share/zoneinfo/Europe/London')

In [5]: Timestamp('2013-10-27 01:00:00').tz_localize('Europe/London', ambiguous=1)
Out[5]: Timestamp('2013-10-27 01:00:00+0100', tz='Europe/London')

In [6]: Timestamp('2013-10-27 01:00:00').tz_localize('dateutil/Europe/London', ambiguous=1)
Out[6]: Timestamp('2013-10-27 01:00:00+0000', tz='dateutil//usr/share/zoneinfo/Europe/London')
```

So on 2.6 looks like the above case is fixed, but the non-ambiguous case is now broken. @pganssle is this on your side? (it could also be on the localization code on pandas side)

```
In [3]: dateutil.__version__
Out[3]: '2.6.0'

In [4]: Timestamp('2013-10-27 01:00:00').tz_localize('Europe/London', ambiguous=0)
Out[4]: Timestamp('2013-10-27 01:00:00+0000', tz='Europe/London')

In [5]: Timestamp('2013-10-27 01:00:00').tz_localize('dateutil/Europe/London', ambiguous=0)
Out[5]: Timestamp('2013-10-27 01:00:00+0100', tz='dateutil//usr/share/zoneinfo/Europe/London')

In [6]: Timestamp('2013-10-27 01:00:00').tz_localize('Europe/London', ambiguous=1)
Out[6]: Timestamp('2013-10-27 01:00:00+0100', tz='Europe/London')

In [7]: Timestamp('2013-10-27 01:00:00').tz_localize('dateutil/Europe/London', ambiguous=1)
Out[7]: Timestamp('2013-10-27 01:00:00+0100', tz='dateutil//usr/share/zoneinfo/Europe/London')
```

@jreback Yes, it seems there is a bug there, probably in the `fromutc()` algorithm, good catch. I will fix upstream, see issue dateutil/dateutil#321. I think it might be related to the fact that the base offset is 0, but I'm not really sure.

If you want to force it to work on 2.6.0, you'll have to determine whether it belongs before or after the fold and use `tz.enfold` appropriately. Otherwise I can probably cut a bugfix release relatively soon.

ok this is updated skipping appropriate sections when 2.6.0 is there (so when you release next version, we will retest).

@jorisvandenbossche because of the segfault, what do you think about 0.19.2 bugfix release?

## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14631?src=pr) is 85.28% (diff: 100%)

> No coverage report found for **master** at 46000da.
> 
> Powered by [Codecov](https://codecov.io?src=pr). Last update [46000da...3f95042](https://codecov.io/gh/pandas-dev/pandas/compare/46000daf257c8e574ca943a57023deac74460edd...3f950426636459c14e17682d578555f38b2c21c0?src=pr)

Error on appveyor that still seems related:

```
======================================================================
FAIL: test_repr (pandas.tseries.tests.test_tslib.TestTimestamp)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "C:\Python27_64\envs\pandas\lib\site-packages\pandas\tseries\tests\test_tslib.py", line 359, in test_repr
    self.assertIn(tz_repr, repr(date_tz))
  File "C:\Python27_64\envs\pandas\lib\site-packages\pandas\util\testing.py", line 922, in assertIn
    assert a in b, "%s: %r is not in %r" % (msg.format(a, b), a, b)
AssertionError: : '/America/Los_Angeles' is not in "Timestamp('2014-03-07 00:00:00-0800', tz='dateutil/US/Pacific')"
```

going to merge this tomorrow unless further comments
can always address in a follow up 

the segfaults on tests are a problem with other prs