-
Notifications
You must be signed in to change notification settings - Fork 8
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
Clarify Temporal RS definition #72
Conversation
The substantive change here is that the spec now makes the ISO8601-based string representation mandatory for the common case of using TemporalRS with a Gregorian calendar. The intention is to make it easier to develop clients that handle this common case, whilst not precluding the possibility of allowing other representations in future. |
Would benefit from a review from @letmaik too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I think this is focused enough to not cause breaks in the next version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good.
@letmaik @jonblower Are you happy to merge now, or would you rather wait until the team meeting 2022-05-18 to give others a chance to consider? |
Ideally it would be great to have input from @marqh with a view to merging early next week if poss? If Mark's not available then maybe we could just give it a final sanity check in the team meeting so people have had a chance to see it. |
@marqh Are you happy with the CoverageJSON backward-compatible compromise wording? Recognising that we will probably do a larger scale alignment of temporal things in the next versions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks a sensible update to me.
one slight query, but i approve
@@ -338,24 +338,23 @@ Example of a vertical CRS, here representing height above the NAV88 datum: | |||
|
|||
### 5.2. Temporal Reference Systems | |||
|
|||
Time is referenced by a temporal reference system (temporal RS). | |||
In this specification, only a string-based notation for time values is defined. | |||
Time is referenced by a temporal reference system (temporal RS). In the current specification, only a string-based notation for time values is defined. Future versions of this specification may allow for alternative notations, such as recording time values as numeric offsets from a given temporal datum (e.g. "days since 1970-01-01"). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't object to this extra sentence, but i'm a little surprised to see a 'future may' included in this section of the specification.
if it's felt to be of value, then please disregard this comment, but i'd slightly stick to the original and leave this as implied, rather than state a conditional future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, and I wondered that too. I added that sentence as it's frequently asked by users, but it could be that the sentence doesn't really belong in a specification, but in some kind of FAQ or guide. Maybe we can discuss on the next call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iagree to leave in the infromative text, as itmakes it easier to read. the text style is really closer to W3C documents rather than ISO
agreed at 2022-05-18 meeting |
Addresses #60