-
Notifications
You must be signed in to change notification settings - Fork 21
New property-based time model. Fixes #499 #619
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
Conversation
Anyone who wants to review in addition to the assigned reviewers, please do! I've added a few |
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'm having a heck of a time figuring out how and where to put comments, hope this is it.
I'm good with changes. In terms of the editorial notes that were in the presentation and didn't make the file, yeah I think that's a good idea.
Also on the question of atDateTime being just gist:dateTime I'm ok with that.
@mkumba @marksem @DanCarey404: @uscholdm and I both think |
I suggest using "OngoingEvent" because a very recent event will generally be regarded a current. The term 'current event' refers to a general time period not whether the event is still happening. E.g. a trial that was just completed, its over but it's in the news and is regarded as a current event. |
@uscholdm I think I agree with you. However, does "ongoing" lend a sense of continuing over some significant period of time? E.g., I am currently stopped at a red light, but I wouldn't call that an ongoing event. Is this too much of a quibble? |
I think the advantage of Contemporary is it contrasts more naturally with Historical. But I'm OK with Contemporary, Ongoing, or Current, whatever others prefer. |
Let's hope the event of discussing this is not ongoing for too much longer :-) |
79b44e5
to
7c62a39
Compare
Question: If the date predicates have precision +/- 12 hours, shouldn't the minute predicates have precision +/- 30 seconds rather than 1 minute? |
Just a voice from the peanut gallery, but it's about time (heh) that this change was made. 👍 |
@bpelakh Since this is a public repository, you would have been welcome to submit a PR! |
I went with the term "contemporary" because I had an essentially boolean category :Contemporaneity. And I had that because there was no real word for "currentness". I prefer contemporary or current, but not ongoing. |
I used :atDateTime versus just :dateTime because the latter looks too much like a datatype, in my opinion. Also, the subproperties I have created in my projects feel more natural with the "at" included. :startAtDateTime, validateAtDateTime, changeAtDateTime, etc. |
@DanCarey404 Regarding the property names, datatype property names are nouns by convention, and |
This property name is definitely not a hill to die on. I just wanted to explain the origin in case anyone found it persuasive. |
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.
See inline. A few key things.
- there is some stuff in this PR that belongs elsewhere - how did it get here?
- need more details in release notes
This property should have been removed as per issue 540. While the other changes proposed in that issue were implemented, the property itself was not removed.
…eTime to gist:atDateTime; fix some subproperty assertions
0855d0c
to
eb794a5
Compare
Replace construct (subClassOf [ intersectionOf X Y]) with simpler (subClassOf X, Y). Tweak a couple of definitions.
All requested changes have been addressed.
Closing this PR because history has gotten screwed up. Have re-opened as PR #629 with a clean history. |
No description provided.