Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functions to get start and end for other kinds of time periods #722

Open
DavidMazarro opened this issue Jun 27, 2022 · 12 comments
Open

Comments

@DavidMazarro
Copy link
Contributor

Following the spirit of #666, it would seem like a good idea to include utility functions like first_day and last_day for similar time periods such as months, quarters and years.

To do this, @sestrella proposes to use a trait and have the corresponding types implement said trait to provide this functionality. For example:

trait Period {
  fn first_in_period(&self) -> NaiveDate;
  fn last_in_period(&self) -> NaiveDate;
  fn period_range(&self) -> RangeInclusive<NaiveDate> {
    self.first_in_period()..=self.last_in_period()
  }
}

impl Period for NaiveWeek { ... } // refactor the existing implementation
impl Period for NaiveMonth { ... }
impl Period for NaiveQuarter { ... }

The implementation of those functions for NaiveWeek would be refactored into an implementation of that trait for NaiveWeek with this new change. Corresponding implementations would be provided for months and quarters as well as for any unmentioned period deemed useful.

Another idea by @sestrella would be to expand this functionality a bit (WRT what's already done for NaiveWeek in #666) and include some sort of "iterator" functionality to the trait. For example:

trait Period {
  // ... previously mentioned functions go here
  fn next_in_period(&self) -> Self;
  fn prev_in_period(&self) -> Self;
}

impl Period for NaiveWeek { ... }
// ... rest of the implementations go here

in this case, using next on Tuesday should yield Wednesday and calling prev on Monday should yield Sunday.

@esheppa
Copy link
Collaborator

esheppa commented Jul 3, 2022

Hi @DavidMazarro - this is an interesting idea, these kind of functions (Especially ones dealing with adding/subtracting months in my experience) have quite wide applicability. That said there are a number of different ways to approach this which will suit different use cases and can generally be built on top of chronos existing API. We may want to discuss whether there are any specific cases which need access to chrono internals and so must be within chrono, or otherwise potentially documenting some crates which implement this kind of functionality, either in the readme, or have a link in the readme to a separate project within the chronotope origination, for example awesome-chrono to aid discoverability (somewhat following ecosystem convention).

One existing crate that has some of this functionality is chronoutil which has a RelativeDuration type which handles adding/subtracting Months and a DateRule type which allows iterating over RelativeDurations.

I also have a (very experimental) library which has a trait that looks quite similar to Period, and generally works for any time period that has a sensible interpretation of next/previous period.

@djc
Copy link
Contributor

djc commented Jul 4, 2022

@DavidMazarro / @sestrella I'd like to talk about use cases. What are you trying to build and where do you run into limitations/gaps in the existing API? #666 seemed like a fairly simple extension but I'd like to explore the design space a bit more starting with the problem instead of a proposed solution to avoid XY problems.

@esheppa
Copy link
Collaborator

esheppa commented Jul 24, 2022

Had some excellent points from @avantgardnerio on this in #731

@esheppa , I'd like to push back against the idea of a YearMonth being related to an instant in time, as proposed in #69 and #152, or at least that being outside the scope of this PR. Just glancing at it, the solution proposed in #184 is a lot more in line with my expectations: YearMonth being a calendar duration, not an instant. I think instant-based YearMonth operations are less common, and Chrono already supports them in methods such as NaiveDate::from_ymd().

If we wanted something more more type-safe than the i32-based solution, I'd propose something like JodaTime's Period. I do think this approach is overly complicated however, since all other durations are true fixed intervals: hours, minutes, seconds, days, and weeks are a defined as number of relative milliseconds (ignoring leap seconds, which even Google does).

Unfortunately the outliers are months and years (which are trivial for users to treat as 12 months). With this being the single edge case, I'd posit that adding a new enum is probably not be worth the complexity: would we need a new NaiveDate.add_calendar_duration() method?

Looking for precedent, it appears both JodaTime and Java 8 support the straightforward solution like the one proposed in this PR:

I'm happy to help do the work to finally get month addition into chrono, but it seems like the first step might be selecting a commonly agreed upon design.

@esheppa
Copy link
Collaborator

esheppa commented Jul 24, 2022

Some previous issues in this repo also discussed similar ideas:

This has also been discussed in chrono-rfcs

I think some things to be considered here are:

  • A potential design for handling YearMonth, adding months, and similar (sometimes referred to as calendar/nominal durations).
  • Whether this should be in chrono or in another crate (potentially within chronotope organization).
    • As per Create add_months() method on NaiveDate #731, it is likely that a basic add_months functionality will be in chrono shortly, and I think this is excellent as it is a frequently requested feature, however it doesn't preclude other newly added functionality (iterators etc) being in a separate crate
  • How this would interact with potentially being generic over calendar systems (potentially this could be solved with a default type parameter.)

My thoughts on one way to implement this are:

  • add_months is useful to have as it is a commonly required operation and so should be in chrono - the use case for it to be added is DataFusion which has to comply with the arrow specification and can't necessarily add a new YearMonth type that might be implemented in chrono, and there are probably many other similar use cases that would benefit from add_months being an operation on NaiveDate. However where users have full control over what types they can use, the documentation could encourage users to instead use a YearMonth style datatype, only converting into a (Naive)Date when needed, the advantage here is that it avoids having to define how to handle month arithmetic and instead leaves that up to converting from a YearMonth to a NaiveDate, and helper methods can be provided to make this convenient.
  • Implement and document the different kinds of datetime types to direct users toward the ones which are most suitable for their use case, where the idea is to use the time that most narrowly represents the requirements to avoid having to use sentinel values for the remaining unneeded accuracy:
    1. Instants in time - this is only NaiveDateTime and DateTime<Tz>
    2. Calendar-style periods representing a specific instance of that period, eg Minute, Hour, Week, Month, Quarter, Year etc. Day and NaiveDate are equivalent. Importantly, these can be advanced forward or back one or n periods similar to succ / pred on NaiveDate, and potentially succ_n and pred_n. Other handy methods could be getting the DateTime or NaiveDate of the start or end of the period, and converting between the different periods, as well as between DateTimes and NaiveDates and the different periods.
    3. Contiguous ranges of the calendar-style periods. These can be iterated over to access the underlying periods. These could also be converted from one to another, eg take the range from 2021-05 through 2021-08 and change the resolution to Day and then iterate. These can potentially be used as a calendar style duration, but importantly, they are always from one specific calendar-style period to another, and never relative.
    4. Accurate durations: Duration from chrono and std
    5. Time only: NaiveTime

@djc
Copy link
Contributor

djc commented Jul 28, 2022

One point to keep in mind is that some periods can be fairly cleanly represented as a Duration (like Minute, Hour, Day, Week) while others can't (Month, Quarter, Year), so we might well want to handle the first group differently from the second group.

@esheppa
Copy link
Collaborator

esheppa commented Jul 28, 2022

My previous comment was probably a bit verbose, but you sum it up well with Month/Quarter/Year being in a different category from Minute and Hour (Day and Week are arguable due to timezone changes and leap seconds)

My attempt at a more succinct explanation of my thoughts is below:

Observations along a (for practical purposes) continuous set of possibilities. These are just NaiveDateTime, DateTime<Tz> and NaiveTime. Duration makes sense here as it is also (for practice purposes) continuous. Due to this continuous set of possibilities, iterators don't make sense here. Examples include:

  • The current time
  • The timestamp of log data

Observations along a discrete set of possibilities at regular intervals (not necessarily all of the same length in time). This includes Months and Quarters and also Minutes, Weeks and Days plus many other reasonable options. I don't think that these should use the existing Duration and instead each could have an associated relative type, for example Months or Quarters which can be Added or Subtracted from the Month or Quarter respectively. These types can all have iterators over ranges (potentially integrating with rust's range syntax) as well as the ability to get the start and end of the period in other resolutions, and also convert between ranges of one resolution to ranges of another. Examples include:

  • The axis of a discrete timeseries, eg monthly economic measurements, five-minute electricity prices, daily closing prices of a commodity derivative
  • A specific re-occurring period, for example the delivery month of a finanicial product
  • Take a Year and then iterate over the contained Minutes

@djc
Copy link
Contributor

djc commented Jul 28, 2022

Quarter reduces down to Months(3), right?

@esheppa
Copy link
Collaborator

esheppa commented Jul 28, 2022

In the provided implementations we could have Quarters(1) == Months(3), Years(1) == Months(12) and avoid having Quarters and Years datatypes. Essentially Month would implement Add<Months>, while Quarter would also implement Add<Months> but there are some cases that wouldn't make sense, for example with Week which would need its own Weeks data type. For users requiring non calendar-year aligned Quarters or Years (eg some Fiscal Quarters / Years), they could make their own implementation which conforms to a common trait that the existing Month, Quarter, etc data types also implement.

@djc
Copy link
Contributor

djc commented Jul 28, 2022

So it's not obvious to me that there's a lot of utility in having Month or Quarter types as IMO these can just be reduced down to NaiveDate.

@esheppa
Copy link
Collaborator

esheppa commented Jul 28, 2022

My main reasons for using types like this are:

  • Storing just the minimal information (eg if all I need is "December 2022" I'd prefer to use a Month data type rather than pick say, 2022-12-01 as a NaiveDate)
  • Iteration over these periods, eg iterate over months between "January 2023" and "December 2023"
  • Conversion of resolution then iterating, eg iterate over the five minute periods between "January 2023" and "December 2023"
  • Conversion of resolution then getting first or last, eg get the last day of "January 2023"
  • Use as keys in BTreeMaps

Admittedly, these may be unusual use cases and may be outside the scope of chrono, especially the resolutions aside from Month.

@djc
Copy link
Contributor

djc commented Jul 28, 2022

Is "the minimal information" a conceptual concern or an actual memory size issue?

@esheppa
Copy link
Collaborator

esheppa commented Jul 29, 2022

Just a conceptual concern. But also practical for example when correlating values from two BTreeMaps with Month keys because I know that the "December 2022" only has one representation, rather than any date from 2022-12-01 through 2022-12-31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants