Naive utc

[klnusbaum]

It seems common to use DateTime<FixedOffset> as a kind of canonical representation of any given instant. With that said, the following code almost works:

fn parse_date(date: &str) -> Result<DateTime<FixedOffset>> {
    let parsed = date
        .parse::<NaiveDate>()?
        .and_time(NaiveTime::default())
        .and_local_timezone(Utc)
        .into()
    Ok(parsed)
}

However, and_local_timezone returns a LocalResult which is kind of irksome in this case. I know the converting to Utc won't produce multiple results, but I still either have to call unwrap or do something more complicated with the LocalResult.

For the case where I know I'm going to Utc, could we just have a special function specifically for Utc? Would folks be opposed to adding the convenience?

[klnusbaum]

I think technically this is semver compatible with 0.4.x but happy to put it on the 0.5 branch if need be.

[klnusbaum]

I added this so that we could have, IMHO, a better example for the and_local_timezone function. However, I can understand if this might be controversial. If we don't want to do this, I can change up the example in and_local_timezone to be something different.

[esheppa]

We could also use

assert!(TypeId::of::<DateTime<Local>>() == dt.type_id());

Which I think would be preferable here as it is hard to interpret PartialEq and Eq on Local.

[klnusbaum]

I'm not sure that would be good given that the purpose of the example (as far as I can tell) is to demonstrate that the created DateTime does indeed have the expected timezone. That would me more of a "see, they have the same type" kind of thing. Which isn't quite the same.

If we don't want to add Eq and PartialEq to Local, then I think the next best thing is to create our own timezone using FixedOffset and use that. I'll put up a change trying that so we can see what it looks like.

[klnusbaum]

waiting on #951 to get merged in order for lints to pass.

[esheppa]

Thanks for this @klnusbaum - this is a good idea, and similar approaches have been discussed elsewhere. Fundamentally, it is painful to have to deal with LocalResult when using FixedOffset, Utc or any other implementer of TimeZone that doesn't ever change its internal offset/ Due to this, there could also be value in somewhat separating the timezone trait such that there can be a generic and_fixed_timezone or similar which works for all timezones which don't change their offset. Let me know your thoughts on this? It does also seem quite reasonable that we could special case Utc here and add this function

[klnusbaum]

there could also be value in somewhat separating the timezone trait such that there can be a generic and_fixed_timezone or similar which works for all timezones which don't change their offset

Ok, I think I'm starting to understand things a little more. So the Offset for a time zone can change (like if there's Daylights Savings time)? Are you saying we could make a new, stricter Timezone trait, maybe call it "FixedTimeZone", that, while implementing everything TimeZone does, also makes stronger guarantees, namely that it's offset never changes (presumeably FixedOffset and Utc would implement FixedTimeZone)? I guess that could work. Though I'd want to think a lot about it. The whole Offset and TimeZone thing is already somewhat difficult to grok IMHO and this could add further confusion. That would also be a much bigger change.

Another option would be adding a function called and_offset which just creates a DateTime from a NaiveDateTime with the desired Offset (which means it would accept a FixedOffset or Utc). Though I'm not sure how much sense that makes (i.e. it would technically work, but does it make sense from a human standpoint).

[esheppa]

Something else to note here is that and_local_timezone was originally added to ease API discovery. The logic being that many of these operations can already be done with methods on the TimeZone trait, but an IDE won't necessarily display these unless the trait has been imported (although these days I think rust-analyzer would). This is the same case for UTC where it is not easily discoverable. I think the best option here is that we just add this and_utc_timezone function as-is, and continue the discussion on the timezone trait over here: #830

With regards to this pull request specifically, please let me know your thoughts on the Eq impl on Local as that's the only thing I'd like to see investigated prior to merging this

[pitdicker]

As a data point, this would make the set of functions to create a DateTime from a NaiveDateTime and TimeZone complete:

• Combine a TimeZone and a local NaiveDateTime:
  • TimeZone::from_local_datetime
  • DateTime<Tz: TimeZone>::from_local
  • NaiveDateTime::and_local_timezone
• Combine a TimeZone and a UTC NaiveDateTime:
  • TimeZone::from_utc_datetime
  • DateTime<Tz: TimeZone>::from_utc
  • NaiveDateTime::and_utc_timezone (missing)

[klnusbaum]

The msrv test failure appears unrelated. I think this is good to merge now.

[pitdicker]

@klnusbaum 👍 I am not really a reviewer yet, but it would be nice if the commits can be squashed, see #898.

#870 had the great suggestion that assume_timezone would be a better name than and_local_timezone. But I suppose that ship is sailed, and and_utc_timezone is the best name for the method in this PR.

[klnusbaum]

squashed commits as suggested

[klnusbaum]

@esheppa does this look good to you?