Skip to content

Commit

Permalink
Export nowutc() from Dates module
Browse files Browse the repository at this point in the history
  • Loading branch information
@quinnj
quinnj committed Aug 27, 2014
1 parent 66f7703 commit 5999f53
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion base/Dates.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export Period, DatePeriod, TimePeriod,
July, August, September, October, November, December,
Jan, Feb, Mar, Apr, May, Jun, Jul, Aug, Sep, Oct, Nov, Dec,
# conversions.jl
unix2datetime, datetime2unix, now, today,
unix2datetime, datetime2unix, now, nowutc, today,
rata2datetime, datetime2rata, julian2datetime, datetime2julian,
# adjusters.jl
firstdayofweek, lastdayofweek,
Expand Down

22 comments on commit 5999f53

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every time I look at this name all I can see is WUT :)

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be some kind of method of now? Or at least call it utcnow.

@quinnj
Copy link
Member Author

@quinnj quinnj commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I thought about a few different things

# string arg
now(x::String="") = ...
# but then only allow "UTC" to be passed? seems kind of lame

# single symbol arg?
now(x::Symbol=:meaningless_symbol) = ...
# similar to string arg, only allow :UTC to be passed?

Alternatively, I guess we could define some of the TimeZone abstracts here and do

abstract TimeZone <: AbstractTime
immutable UTC <: TimeZone end
now() = ... # get user's local time; as it is now
now(::Type{UTC}) = ... # get time in UTC

This last one is actually appealing now that I think about it since I was going to provide those definitions in TimeZones.jl anyway.

@quinnj
Copy link
Member Author

@quinnj quinnj commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the only awkward part of the last suggestion is that now is exported to top level, whereas I wasn't thinking UTC would, So calling it would be now(Dates.UTC). Maybe it's not that big a deal though.

@ivarne
Copy link
Member

@ivarne ivarne commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So now(Dates.UTC) will work, but now(Dates.EST) will require the TimeZones package?

@quinnj
Copy link
Member Author

@quinnj quinnj commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the now(Dates.UTC) interface but it is weird that now(Dates.EST) wouldn't work without hacking timezones into the Dates module after the fact. Maybe UTC could be exported? Or maybe it's ok for the TimeZones package to jack definitions back into Dates.

@quinnj
Copy link
Member Author

@quinnj quinnj commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it could be now(TimeZones.EST) and TimeZones wouldn't have to jack anything into the Dates module. There could even be a duplicate TimeZones.UTC.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That might be the best option. And if you do using TimeZones then you get all of the time zones without prefixes.

@ivarne
Copy link
Member

@ivarne ivarne commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather keep nowutc, or rename it to utcnow. Both now() and utcnow() are kind of timezone agnostic (depends on system settings), and it seems strange to have only one timezone available in Dates.

@nalimilan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OTC I'd prefer having a single now function, it's more consistent and it makes it easier to find the relevant documentation. I don't see it as a problem that if you didn't load the TimeZones packages, you can only select one time zone: after all, that's what the package is for, and you didn't load it!

@ssfrr
Copy link
Contributor

@ssfrr ssfrr commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also like now() and now(tz::TimeZone) methods. I'm OK with Dates only supporting 1 time zone (UTC) and TimeZones for the rest. The fact that getting UTC time uses a different mechanism under the hood seems like an implementation detail.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about having offsets from UTC instead of time zones? I.e. You can use any offset from UTC as a pseudo time zone, and now defaults to wherever you currently are. Then now(0) would give you UTC time without needing a time zone object.

@ivarne
Copy link
Member

@ivarne ivarne commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution!
, but is it obvious enough that now(0) and now() gives different answers if your current local time is different from UTC?

@nalimilan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@StefanKarpinski That wouldn't be enough, since time zones can change their offsets depending on the period of the year, e.g. with DST. And I can never remember what's my offset from UTC, while I sure know my time zone. :-)

Though both APIs could be supported, of course.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the point – you can use real time zones if you load the TimeZones package but you can get UTC without knowing about time zones. I guess you would just have to document that this is how you get a UTC time without having to load the TimeZones package.

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a great idea to have Base.Dates include one timezone (UTC), and use timezones in APIs where appropriate. That way there is one general interface, and the TimeZones package provides the extra timezones without changing how things work.

And one last plaintive plea for timezones to be instances, and not types. Dumping in an extra 419 types is driving the compiler crazy. It appears to be common to look these up in Dicts. Anything can be a Dict key; they do not have to be types. I realize one reason for this is that as instances they couldn't be used as type parameters, but I'm willing to allow that.

@StefanKarpinski
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Jeff is spot on – define UTC in Base. It's not like that's going to be a popular name for variables. And yes, can time zones pretty, pretty please be values not types? We can allow arbitrary immutable values as type parameters and then there's no reason for them to be types.

@quinnj
Copy link
Member Author

@quinnj quinnj commented on 5999f53 Aug 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image

@nalimilan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't time zones the epitome of an enum? Whatever the enum type in Julia becomes, it seems time zones should use it.

@JeffBezanson
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I agree with that. But enum values will definitely be instances and not subtypes :)

@nalimilan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Please sign in to comment.