0.5: Convert API to return Result: Datelike::with_year

[Zomtir]

Follow up commit for #1444. Split from #1445.

• Converts Datelike::with_year to return Result<T,E>
• Converts the implementations NaiveDate::with_year and NaiveDateTime::with_year to return Result<T,E>
• Converts the implementation DateTime<Tz>::with_year to return Result<T,E> as well
  • Adds DateTime<Tz>::map_local_result as transitionary helper function to prevent dependency on DateTime<Tz>::map_local
  • Adds LocalResult<T>::map_result_unique to have a transitionary, centralized mapping function until 0.5.x: Timezone conversion result (LocalResult) #1448 is settled

The function map_result_unique is NOT meant to outline future behaviour in any form. The idea is that there is some centralized mapping to a Result that can easily tracked for the purpose of #1448. Local (non-central) mappings will be prone to spread inconsistent behaviour.

However the future of LocalResult looks like, it will be easy to track the current usage and adapt the implementation and documentation by searching for the map_result_unique usage. For that reason I did not document the behaviour of map_result_unique anywhere else than in src/offset/mod.rs, but referenced it instead.

cc @pitdicker

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.03%. Comparing base (2e4e6ed) to head (c55ba4f).

Additional details and impacted files

@@            Coverage Diff             @@
##            0.5.x    #1466      +/-   ##
==========================================
- Coverage   94.04%   94.03%   -0.01%     
==========================================
  Files          37       37              
  Lines       17022    17005      -17     
==========================================
- Hits        16008    15991      -17     
  Misses       1014     1014              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pitdicker]

@Zomtir Thank you!
Why do you keep trying to work on something that I consider one of the last pieces to convert, and don't pick something that is easy and ready without needing workarounds? I keep suggesting it friendly:

From #1444 (comment)

It seems good to do the conversion with only one method or a very small number of methods per commit. And starting with methods in the naive module that don't depend on pieces that have not yet been converted.

Converting LocalResult, ParseError and TimeDelta require some preparations first that I'll look into. Let's ignore anything around those for now.

From #1444 (comment)

Could you skip the DateTime type? The map_local type is going to pass on multiple error causes, such as OutOfRange, DoesNotExist, OS errors, and trouble with a TZ string or TZif file. I'd like to be a bit more careful with that type and leave that to its own PR.

We should also have a discussion around the Datelike and Timelike traits before converting them. For the naive types their signature is fine. For DateTime it would be better if they returned LocalResult to give users a way to deal with timezone issues (in my opinion). See #1050.

Repeated in #1445 (comment).

From #1445 (comment)

That is not addressing the problem. I cannot change the signature of an implementation without changing the signature of the trait.

True. That is why I suggested not touching the trait in this PR yet, and wait a bit.

And in #1445 (comment)

May I suggest the following methods as a first step ...

[Zomtir]

@pitdicker I have neither issue with the pull request idling until it gets useful and/or rebasing it once changes are made. Don't worry.

I am looking into from_num_days_from_ce as we speak and will leave a comment on #1445.

[pitdicker]

With #1490 these methods are no longer part of a trait and can be adjusted one type at a time.
Now is a good time to work on the methods on NaiveDate, NaiveTime and NaiveDateTime.
I'd still like to avoid DateTime.

[Zomtir]

I rebased the changes onto the the current 0.5.x. Now only NaiveDate and NaiveDateTime are affected.

• API change of with_year to return Result
• Linking is now more prominent in the documentation to follow the 'DRY' principle. There is little use in repeating the same information when the function has no own logic (e.g. NaiveDateTime::with_year directly calls NaiveDate::with_year).
• For the same reason the examples in NaiveDateTime::with_year now only provides an example where the NaiveDateTime is used.

CC @pitdicker

[pitdicker]

A couple of comments. I think it is mostly caused by the rebase?

[pitdicker]

Very nice!