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

Fix the implementation of recurrence format number 1 #45

Closed
benfitzpatrick opened this issue Jun 4, 2014 · 4 comments · Fixed by #183
Closed

Fix the implementation of recurrence format number 1 #45

benfitzpatrick opened this issue Jun 4, 2014 · 4 comments · Fixed by #183
Assignees
Labels
Milestone

Comments

@benfitzpatrick
Copy link
Contributor

benfitzpatrick commented Jun 4, 2014

The first recurrence format does not define repetitions within the given start and end date - it actually just extracts the interval between the start and end date and repeats that starting at the start date. We need to alter the code to represent this.

@benfitzpatrick benfitzpatrick added this to the soon milestone Jun 4, 2014
@benfitzpatrick benfitzpatrick self-assigned this Jun 4, 2014
@arjclark arjclark modified the milestones: soon, some-day Dec 2, 2016
@matthewrmshin matthewrmshin modified the milestones: some-day , soon Jan 23, 2019
@MetRonnie MetRonnie changed the title Fix the recurrence format number 1 definition Fix the implementation of recurrence format number 1 Jun 19, 2020
@MetRonnie MetRonnie self-assigned this Jun 19, 2020
@MetRonnie
Copy link
Contributor

MetRonnie commented Jun 19, 2020

The current incorrect implementation also leads to a TypeError if the repetitions arg is omitted when creating a TimeRecurrence instance of format 1, e.g.

TimeRecurrence(start_point=TimePoint(year=2020), end_point=TimePoint(year=2022))

@MetRonnie MetRonnie assigned MetRonnie and unassigned MetRonnie Sep 10, 2020
@MetRonnie
Copy link
Contributor

To give an example of and clarify how our implementation of recurrence fmt 1 is wrong, take the following TimeRecurrence:

R4/2004-01-01/2004-01-02

According to ISO 8601 this should be a recurrence starting at 2004-01-01T00, with an interval duration of 1 day (the difference between the two date-times given) and so ends at 2004-01-04. This is functionally equivalent to

R4/2004-01-01/P1D

However, in our incorrect (but unfortunately longstanding) implementation, the example above means a recurrence starting at the same date-time but ending at 2004-01-02T00 with an interval duration of 6 hours.

@MetRonnie
Copy link
Contributor

MetRonnie commented Sep 14, 2020

Quote from ISO/WD 8601-1 (ISO/TC 154/WG 5 N0038, 2016)

4.5.1 Means of specifying recurring time intervals

A recurring time interval shall be expressed in one of the following ways.

  • a) By a number of recurrences (optional), a start and an end. This represents a recurring time interval of which the first time interval is identified by the last two components of the expression and the number of recurrences by the first component. If the number of recurrences is absent, the number of occurrences is unbounded.
  • b) [Not implemented in metomi-isodatetime]
  • c) By a number of recurrences (optional), a start and a duration. This represents a recurring time interval of which the first time interval is identified by the last two components of the expression and the number of recurrences by the first component. If the number of recurrences is absent, the number of occurrences is unbounded.
  • d) By a number of recurrences (optional), a duration and an end. This represents a recurring time interval of which the last time interval is identified by the last two components of the expression and the number of recurrences by the first component. If the number of recurrences is absent, the number of occurrences is unbounded.

(emphasis mine)

@MetRonnie
Copy link
Contributor

MetRonnie commented Sep 14, 2020

Another thing: the current implementation suffers from floating point errors due to dividing the difference between start_time and end_time by the number of repetitions to get the interval duration. This makes the comparison operations tricky

MetRonnie added a commit to MetRonnie/isodatetime that referenced this issue Sep 17, 2020
Updated/added relevant tests
@MetRonnie MetRonnie linked a pull request Sep 17, 2020 that will close this issue
2 tasks
MetRonnie added a commit to MetRonnie/isodatetime that referenced this issue Oct 8, 2020
Updated/added relevant tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants