Add parse_from_xxx to DateTime<Utc>

[mqudsi]

As discussed in #263. Note that this breaks existing code that did not
explicitly specify the offset in calls to DateTime::parse_from_*, as
they are now ambiguous.

Relies on #271 for conversion back into DateTime<Utc> from
DateTime<FixedOffset> as the latter is used under the hood.

[djc]

Makes sense to me, I think this is helpful new API. @esheppa any thoughts?

[djc]

Nit: would like these doc strings to start with a single concise line describing the functionality, then an empty line, then a longer/more detailed description.

[mqudsi]

I agree. These were copy and pasted from the original impl for FixedOffset at the time the patch was first authored - I don’t know if those impls have had their docs updated but I agree that it would be the correct way given the way rust docs are generated and displayed and given that it’s the defacto approach the community has adopted.

I’ll update all the docs in this PR accordingly and squash the changes.

[djc]

Cool, yeah, if you update the docs, let's merge this!

Note: this PR targets main but main is now intended for 0.5.0 development. Since this is additive, we could merge it into 0.4.x to make it available sooner, so maybe rebase and change the PR's target branch?

[esheppa]

My only thought is whether this should assert that the timezone is UTC (internally, this can be done by parsing as a FixedOffset and verifying that local_minus_utc() is 0)? It is arguable either way, but it might be confusing if a time is parsed that has an offset, only for the timestamp to change as it is converted to UTC.

[mqudsi]

@esheppa I respectfully disagree. The strongly typed type declared in the code is how one wants the date and time to be stored. The kind of date that it comes from is specified by the input format and can diverge without a problem.

eg someone wants all datetime variables to be in UTC because that’s what they want to deal with and enforce is stored in the db, on the disk, displayed (unless converted), etc. But as long as I can convert from a non-UTC datetime losslessly, I would absolutely hate to be blocked from using that.

In this specific instance, the function names indicate what input formats are allowed. In this case, the input should be any valid RFC 2822 or RFC 3339 string representing a complete date & time, neither of which are restricted to UTC-only.

If this were a “deserialize” impl (reversing the output of a serialized UTC datetime) perhaps you could get away with requiring (for no measurable benefit) that datetime strings are all in UTC but that’s not the point of this question - and even in that case, I don’t see the point in not being flexible and forgiving in what we allow as input.

The function signatures tell you what to reasonably expect: any valid RFC 2822/3339 text input and a DateTime<Utc> output. It would make no sense to arbitrarily restrict the inputs to a subset of what the RFC supports and I would argue that if we had code that did that, sooner or later someone would rightly file a bug complaining about it.

[esheppa]

Thanks for your reply @mqudsi - that sounds reasonable to me - and from a second look at the docs it looks like there are a few examples that show this

[mqudsi]

I updated the branch with the requested changes to the documentation. Note that I didn't squash and force push because it turns out the docs for the DateTime<FixedOffset> parsing functions were also incorrectly formatted and imprecise, so I decided to just tackle both in one go.

I'm happy to open a separate PR targeting 0.4.x with these commits rebased on top of the current 0.4 branch but I want to confirm that the API breakage is ok for a 0.4.x release? While the changes are purely additive, any existing usage of DateTime::parse_xxx(...) would break as that is now an ambiguous method call that may return either a DateTime<Utc> or a DateTime<FixedOffset> value (I honestly have no idea why these were only implemented for DateTime<FixedOffset> in the first place).

[djc]

Ah yeah, in that case it's probably better to stick with 0.5.x only.