Utc.timestamp method is unintuitive

[kornelski]

I've been looking for a way to get DateTime<Utc> from a unix timestamp, and fudged this monstrosity:

DateTime::from_utc(NaiveDateTime::from_timestamp(timestamp as _, 0), Utc)

only to discover later that Utc.timestamp() exists too. But this method was hard to find:

• It's called timestamp, not from_timestamp like the other conversion methods.
• It's an instance method, not a static method Utc::from_timestamp like most constructors in Rust.
• It's naming suggests it returns a timestamp (like a getter), but it returns DateTime.

[esheppa]

Thanks for raising this @kornelski. Some of the solutions I've suggested in #831 probably apply here as well, but its worth going through a few of these points to explain why the current API works the way it does:

It's called timestamp, not from_timestamp like the other conversion methods.

This is likely because all these methods take self by reference, and it requires silencing a clippy lint to prefix the method with from_ - however making these all consistent does seem to be an improvement.

It's an instance method, not a static method Utc::from_timestamp like most constructors in Rust.

While this may seem superfluous when using Utc, any TimeZone implementer that maintains internal state, eg FixedOffset, requires the reference to self to get at that state

It's naming suggests it returns a timestamp (like a getter), but it returns DateTime.

Yes it does seem that this could be better named - I'm open to any suggestions you may have, some thoughts are:

• (as an inherent method on impl DateTime<Utc>) - fn from_timestamp(secs: i64, nsecs: u32) -> DateTime<Utc>
• rename TimeZone method: fn create_from_timestamp(&self, secs: i64, nsecs: u32) -> DateTime<Self>
• rename TimeZone method: fn datetime_from_timestamp(&self, secs: i64, nsecs: u32) -> DateTime<Self>
• rename TimeZone method: fn from_timestamp(&self, secs: i64, nsecs: u32) -> DateTime<Self> (has the disadvantage of being against the convention of taking self by value when prefixed with from)

My overall thinking here is that there is a decent usability improvement by making the traits as minimal as possible and moving as much as possible to inherent methods on the types like DateTime, Utc, etc.

[kornelski]

Thank you for your response.

Utc.datetime_from_timestamp would be nice. The datetime there is to signify it doesn't create Utc, and it would also allow date_from_timestamp.

But having these methods on Utc singleton is odd. When I need a DateTime, I look for DateTime::from* methods, which is why I've stumbled into DateTime::from_utc in the first place. So DateTime::from_timestamp would be ideal.

[djc]

I think it makes sense to add DateTime::from_timestamp() for this, and ignore any clippy complaints that come up.

[demurgos]

I definitely agree with this issue. When searching for a conversion function, I scanned the documentation for new and from_* functions. It's easy to overlook the timestamp function. The suggestion by @djc to use from_timestamp would be very helpful.

When searching for exising issues in this repo, I also found that this issue was already raised a couple of times before:

• Easy way to convert timestamp to UTC object? #88
• Converting a timestamp to local time is verbose and unintuitive #200

[djc]

@demurgos want to send a PR?

[demurgos]

Sure, here it is: #1278 #1279

[pitdicker]

#1279 added DateTime::<Utc>::from_timestamp, which partly fixes this issue.

I would like to keep this open to track renaming TimeZone::timestamp.