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

Representing Exception TimeSpans #117

Open
josh-p-thompson opened this issue Apr 12, 2023 · 5 comments · May be fixed by #136
Open

Representing Exception TimeSpans #117

josh-p-thompson opened this issue Apr 12, 2023 · 5 comments · May be fixed by #136
Assignees
Labels
Curbs API minor update A change that is minor and should require little discussion, or is a maintenance/readme/typo update.
Milestone

Comments

@josh-p-thompson
Copy link

josh-p-thompson commented Apr 12, 2023

Is your feature request related to a problem? Please describe.

There is no way to define and communicate an exception Time Span that includes periods of time, based on our reading of the Time Span spec (exception periods are "externally-defined").

Describe the solution you'd like

We would like

  • designated_period_except = true to indicate that a rule does not apply during the defined Time Span
  • Time Span's with designated_period_except = true always supersede those with designated_period_except = false | null

Example 1: Time Span to communicate that a rule does not apply on Sundays:

{
    'days_of_week': ['sun'],
    'designated_period_except': true, 
}

Example 2: Time Span to communicate that a rule does not apply on New Years Day:

{
    'days_of_mont': [1],
    'months': [1],
    'designated_period': "New Year's Day",
    'designated_period_except': true, 
}

Is this a breaking change

A breaking change would require consumers or implementors of an API to modify their code for it to continue to function (ex: renaming of a required field or the change in data type of an existing field). A non-breaking change would allow existing code to continue to function (ex: addition of an optional field or the creation of a new optional endpoint).

  • I'm not sure (edit: no)

Impacted Spec

For which spec is this feature being requested?

  • Curbs

Describe alternatives you've considered

There does not appear to be any other way to communicate the actual time period of the exception Time Span.

Additional context

@jlarsonOmahaNE jlarsonOmahaNE added this to the 1.0.1 milestone May 30, 2023
@schnuerle
Copy link
Member

We will be talking about this at next week's Working Group meeting for inclusion in a forthcoming CDS 1.0.1 patch release, so please attend if you can.

@LaurentG-AMD
Copy link

How did the discussion go? I was about to open the same issue when I saw this one since most regulations in Montreal are written as "No Parking except between X-Y" and this would definetly complicate updating the information later one if every regulation has to have it's logic reversed.

@schnuerle
Copy link
Member

The meeting didn't lead to any concerns about adding this as a patch in the upcoming 1.0.1 release. Feel free to make a PR and speed this along. We may bring it up in tomorrow's end of year event.

@schnuerle
Copy link
Member

If anyone wants to do a PR for this, that would be welcome and you and your org will be recognized in the acknowledgements for 1.0.1. Otherwise the OMF staff will make a PR soon.

@schnuerle schnuerle added the minor update A change that is minor and should require little discussion, or is a maintenance/readme/typo update. label Mar 9, 2024
@mplsmitch mplsmitch linked a pull request Mar 11, 2024 that will close this issue
@schnuerle schnuerle linked a pull request Mar 26, 2024 that will close this issue
@schnuerle
Copy link
Member

schnuerle commented Jun 3, 2024

There is a PR now #136. The question is, based on the comments there, is this a breaking change or not? Please look at the PR and discuss.

Also see issue #124 for a similar discussion. We should decide how to execute on this to determine if both issues and be addressed in a patch or a minor release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Curbs API minor update A change that is minor and should require little discussion, or is a maintenance/readme/typo update.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants