Pre-PR: Please review year/month Duration, date iteration

[kosta]

trafficstars

Hi!

I wrote a small library that provides the following on top of chrono:

• CalendarDuration that is able to add months and years (which have varying lengths and thus cannot be represented as seconds) - fixes #52
  • This allows humantime to meaningfully parse months and years and not decide that one month = 30.5 days or so - makes fixing #129 easy
• date_iterators that can be used to iterate over date ranges - fixes #152

https://github.com/kosta/date-iterator

This has some rough edges as some stuff can only be implemented in chrono itself, e.g. I cannot do impl<Tz: TimeZone> Add<CalendarDuration> for DateTime<Tz> outside of chrono.

Please let me know whether you like this general direction and would consider merging such functionality in a PR. In that case I would prepare a branch for merging. I'm also happy to discuss & adjust any design choices.

Thanks in advance for your consideration! :)

Cheers, Kosta

[kosta]

Did you get a chace to take a look? :) Maybe it makes more sense to chat about this?

[quodlibetor]

Thanks for this! I really want this kind of functionality in chrono, there are some open questions about what the best design should look like. In particular we are thinking of adding a YearMonth enum to get rid of raw ints for months and to enable some nicer APIs.

I haven't looked too deeply at your code, but it appears to be more "duration-focused" than "instant-focused", if that makes sense? I am certain that we would need something like your CalendarDuration to be able to implement all the iterators that we want, but I'm not sure if our desire to change the chrono data model influences your opinion of what a good duration and iterator API would look like?

I'll take a closer look at your code over the weekend, and yes I will be happy to chat about it! I'm very excited that you're interested in this!

[kosta]

Thank you for taking a look! Yes, this is totally duration-focused.

If you're interested, I can merge this into a chrono fork, just to see what it would look like. This would allow to see how the API can be improved by this being part of chrono.

Let's chat next week :)

[quodlibetor]

If you don't mind the possibility that we'll refactor heavily then I think that yes, merging into a chrono fork makes sense. Feel free to try and improve the chrono data model to make it easier to implement! I really want some direct experience of what we can do to make the entire project easier to work with.

[kosta]

Cool, I hope I can whip something up early next week!

[kosta]

I tried a more logical approach in the date-op branch. It's not 100% done yet but I think this makes the most sense:

https://github.com/kosta/chrono/tree/date-op (All the changes from the current date-iterator branch are in this commit: https://github.com/kosta/chrono/commit/af52b1b09e1799c9da427e740139fa4f42c8dc20)

It takes care of both issues you opened https://github.com/kosta/date-iterator/issues/1 and https://github.com/kosta/date-iterator/issues/2

The date-iterators stayed basically the same, but CalendarDuration is replaced by the DateOp interface.

It needs more tests, docs and a special-case handling for MonthDuration(..., Skip) in the date-iterators plus maybe a few more ops (end of day/month/year; maybe event setDay/month/year etc.).

But apart from that, I think this should work out. What do you think?

CC'ing mayurdzk for their interest in this functionality

(Edit: If it's looks kinda-ok, I will merge it into this branch to update this PR)

[peckpeck]

why only month and years, you also should add week and days, since they also have varying duration because of dst.

This would interest me

[pitdicker]

We now have a dedicated issue on adding a CalendarDuration type with these features: https://github.com/chronotope/chrono/issues/1282.