Support converting between DateTime and ZonedDateTime interval types

[rofinn]

Currently, if you want to convert an interval of DateTimes to a ZonedDateTime you need to do something like:

function _to_zdt(i::AnchoredInterval{P, DateTime, L, R}, tz::TimeZone) where {P,L,R}
    return AnchoredInterval{P, ZonedDateTime, L, R}(ZonedDateTime(anchor(i), tz))
end

I'm not sure if overloading the DateTime and ZonedDateTime constructors makes sense here, but these conversions should probably be handled in this package given that they're pretty common operations and we're already depending on TimeZones.jl.

[omus]

As moving between DateTime and ZonedDateTime isn't lossless there definitely needs to be some user input to make this transition. Could you provide more context into why you want this operation? I suspect this may influence some design in TimeZones.jl more than Intervals.jl

[rofinn]

Our use case is that we need to work around JuliaTime/TimeZones.jl#271. Since we have large arrays of timestamp intervals with the same timezone we can just determine that timezone by sampling the file. Only parsing the datetime component is faster and more space efficient. To avoid breaking internally APIs we add the timezone back, only when queried.

[omus]

Our use case is that we need to work around JuliaTime/TimeZones.jl#271

It would be great to find a path forward on that. As you're already working with switching to using DateTime why don't you make:

struct UTCDateTime <: AbstractZonedDateTime
    utc::DateTime
end

This avoids having to implicitly keep track of the DateTime time zone and avoids the isbits problem. The main challenges here is we'd need to define AbstractZonedDateTime and your internal APIs would also need to support this.

Finally, even with UTCDateTime the convert interface leaves something to be desired. I think this is another good example for JuliaTime/TimeZones.jl#318 and could eventually be done with:

convert(AnchoredInterval{ZonedDateTime{TZ"America/New_York"}}, interval::AnchoredInterval{UTCDateTime})
convert(AnchoredInterval{UTCDateTime}, interval::AnchoredInterval{ZonedDateTime})

[rofinn]

The main challenges here is we'd need to define AbstractZonedDateTime and your internal APIs would also need to support this.

Yeah, that's the main reason why I didn't want to get into it. In this particular case, we don't really need a more general solution, and even the UTCDateTime solution would require more significant updates. I agree that in the future we'll probably want to go that direction, but for the couple hundred LOC in which we're tracking the timezone I don't think it's a priority.

FWIW, I think these conversions should be supported regardless of whether the isbits issue is fixed. We already support astimezone here anyways.

[omus]

Yeah, that's the main reason why I didn't want to get into it.

Fair enough. It helps seeing the bigger picture to figure out a better path forward.

FWIW, I think these conversions should be supported regardless of whether the isbits issue is fixed. We already support astimezone here anyways.

The astimezone methods here have always felt out of place but I can't dispute they are convenient with the current interface. The proposed _to_zdt seems even more out of place and maybe should live near these couple hundred LOC for now. Just my opinion though.

[rofinn]

The proposed _to_zdt seems even more out of place and maybe should live near these couple hundred LOC for now.

Yeah, that's what we're doing for now, just opened this issue more for reference. If we want to drop the astimezone stuff, then I think it's fine to close this issue. It would be nice if there was a convert interface that allowed you to generically convert the elements. Kind of like how parse takes an element_parser.