Open rofinn opened 3 years ago
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
Our use case is that we need to work around https://github.com/JuliaTime/TimeZones.jl/issues/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.
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 https://github.com/JuliaTime/TimeZones.jl/issues/318 and could eventually be done with:
convert(AnchoredInterval{ZonedDateTime{TZ"America/New_York"}}, interval::AnchoredInterval{UTCDateTime})
convert(AnchoredInterval{UTCDateTime}, interval::AnchoredInterval{ZonedDateTime})
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.
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 supportastimezone
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.
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
.
Currently, if you want to convert an interval of
DateTime
s to aZonedDateTime
you need to do something like:I'm not sure if overloading the
DateTime
andZonedDateTime
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.