search

JuliaTime / TimeZones.jl

IANA time zone database access for the Julia programming language
Other
85 stars 54 forks source link

Constructor: ZonedDateTime(::Date, ::Time, timezone) #268

Open oxinabox opened 4 years ago

oxinabox commented 4 years ago

Right now we accept: ZonedDateTime(::Date. timezone), and ZonedDateTime(::DateTime, timezone)

I suggest that we should accept all the common constructors for DateTime. Which we can probably do by slurping up arguements and just passing all but the last 1 or two onwards. Alternatively we could stop accepting anything other than a DateTime to give that argument

Example, the following works:

julia> ZonedDateTime(DateTime(Date(now()), Time(now())), tz"America/Winnipeg")
2020-06-23T13:44:25.743-05:00

But this does not:

julia> ZonedDateTime(Date(now()), Time(now()), tz"America/Winnipeg")
ERROR: MethodError: no method matching ZonedDateTime(::DateTime, ::Time, ::VariableTimeZone)
omus commented 4 years ago

I'm not sure about adding this as Date + Time already exists.

I suggest that we should accept all the common constructors for DateTime.

Could you define what you mean by "common"? I wouldn't consider the DateTime(dt::Date, t::Time) constructor to be used very often and there are several other DateTime constructors I wouldn't want to introduce.

Alternatively we could stop accepting anything other than a DateTime to give that argument

Dropping the ZonedDateTime(y, m, d, ...) constructor would be a major inconvenience with no upside from what I can see.

oxinabox commented 4 years ago

Could you define what you mean by "common"? I wouldn't consider the DateTime(dt::Date, t::Time) constructor to be used very often and there are several other DateTime constructors I wouldn't want to introduce.

Common i was thinking as: all the ones in Dates stdlib (as opposed to ones defined in packages/other standard libraries). Though I admit some of those are annoyingly obsure/weird.

But just having all of them means that ZonedDateTime is a almost drop in replacement for DateTime.

I don't feel particularly strongly about this.

I expected the ZonedDateTime(::Date, ::Time, ::TimeZone) to work, i thinki in part becuase of the name, It is a Zoned Date Time and I gave it a [Time]Zone a Date, and a Time