Skip to content

Issue 499 cleanups. #615

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

Issue 499 cleanups. #615

wants to merge 4 commits into from

Conversation

marksem
Copy link
Collaborator

@marksem marksem commented Jan 19, 2022

Rebecca, This is kinds of a "test" PR. I'd like to walk through these diffs with you to before we try to merge.

@rjyounes rjyounes changed the title Issue 499 cleanups Issue 499 cleanups. Fixes #499. Jan 25, 2022
rjyounes
rjyounes reviewed Jan 25, 2022
View reviewed changes
gistCore.ttl
@@ -402,14 +402,14 @@ gist:ContemporaneousEvent
gist:Event
[
a owl:Restriction ;
owl:onProperty gist:hasActualStart ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these are datatype properties, they need to be non-verbal: actualStart, etc.

rjyounes
rjyounes reviewed Jan 25, 2022
View reviewed changes
gistCore.ttl
owl:onProperty gist:hasActualStart ;
owl:someValuesFrom gist:TimeInstant ;
owl:onProperty gist:startAtDateTime ;
owl:someValuesFrom xsd:dateTime ;
Copy link
Collaborator

@rjyounes rjyounes Jan 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a personal aversion to this construction, since it makes it appear that xsd:dateTime is not the range of startAtDateTime, when in fact it is. I know Michael prefers someValuesFrom to cardinality, but my preference is to use a cardinality constraint when the type of object doesn't need to be specified.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend these property names:

actualEnd

Subproperties:

actualEndDate
actualEndMinute
actualEndSystemTime

and the parallel properties for planned end, actual start, and planned start.

@rjyounes rjyounes changed the title Issue 499 cleanups. Fixes #499. Issue 499 cleanups. Jan 25, 2022
@mkumba
Copy link
Contributor

mkumba commented Jan 25, 2022 via email

@mkumba
Copy link
Contributor

mkumba commented Jan 25, 2022 via email

@marksem
Copy link
Collaborator Author

marksem commented Jan 25, 2022

I think the real problem is this still has gist:TimeInstant. I thought I got rid of all those, my bad.

I believe all TimeInstants are gone. That was just showing a deletion line, I think.
However, as I checked this, I did notice that TimeInstance is still in 2 annotations. I can remove it from those places.

@marksem marksem force-pushed the issue-499-cleanups branch from f3ceee2 to 7bf43c7 Compare January 25, 2022 18:03
@marksem marksem force-pushed the issue-499-cleanups branch from 7bf43c7 to 8b00c4f Compare January 25, 2022 18:06
@rjyounes
Copy link
Collaborator

@mkumba @marksem FYI I have not finished my review yet.

rjyounes
rjyounes requested changes Jan 25, 2022
View reviewed changes
Copy link
Collaborator

@rjyounes rjyounes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main issues I see are:

  1. We need to settle on the property names. Since Dave doesn't seem to care, I think we can determine these ourselves based on our conventions.
  2. A lot of annotations need to be cleaned up and reworded.
  3. There are extraneous modifications that need to be added to a different issue/PR.

@marksem We can walk through this together whenever you're ready.

gistCore.ttl
@@ -10,7 +11,6 @@
a owl:Ontology ;
owl:versionIRI <https://ontologies.semanticarts.com/o/gistCoreX.x.x> ;
skos:definition "gist is a minimalist upper ontology created by Semantic Arts"^^xsd:string ;
skos:prefLabel "gist Core"^^xsd:string ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this being deleted?

gistCore.ttl
owl:onProperty gist:hasActualStart ;
owl:someValuesFrom gist:TimeInstant ;
owl:onProperty gist:startAtDateTime ;
owl:someValuesFrom xsd:dateTime ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend these property names:

actualEnd

Subproperties:

actualEndDate
actualEndMinute
actualEndSystemTime

and the parallel properties for planned end, actual start, and planned start.

gistCore.ttl
]
[
a owl:Restriction ;
owl:onProperty gist:hasActualEnd ;
owl:onClass gist:TimeInstant ;
owl:onProperty gist:endAtDateTime ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change ContemporaneousEvent to ContemporaryEvent since contemporaneous means something different. I agree with the qualified cardinality restriction.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change ContemporaneousEvent to ContemporaryEvent since contemporaneous means something different.

According to https://english.stackexchange.com/questions/30207/contemporary-vs-contemporaneous, "Contemporary usually applies to people or small groups of people. For example, the Beatles and the Beach Boys were contemporary with each other because they were active at roughly the same time. Contemporaneous usually applies to events, movements, or trends. For example, the rise of rock music was contemporaneous with the economic boom and counterculture movements of the 1950s and ‘60s."

This would suggest that ContemporaneousEvent is correct. One might argue that there needs to be two things to be contemporaneous, but perhaps the same is true with contemporary? In any event, (so to speak) we can think of a ContemporaneousEvent as being at the same time as "now". Maybe CurrentEvent is better?

Copy link
Collaborator

@rjyounes rjyounes Jan 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contemporaneous is a relationship between two things. Contemporary as an adjective is a property of a single thing. I disagree with the definition of "contemporary" that you refer to; the example cited is not even grammatical: "For example, the Beatles and the Beach Boys were contemporary with each other." This should be "The Beatles and the Beach Boys were contemporaries [of one another]" or "The Beach Boys were the Beatles's contemporaries" using the nominal "contemporary."

The sense of "contemporary" I am familiar with is "belonging to or occurring in the present." Think of contemporary art, contemporary culture, contemporary film, contemporary mores, ...

gistCore.ttl
owl:onProperty gist:hasBiologicalParent ;
owl:someValuesFrom gist:LivingThing ;
owl:onProperty gist:birthDate ;
owl:someValuesFrom xsd:dateTime ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't like this. there is one and only one birthdate. birthDate is a subproperty of startDate so range is provided without the someValuesFrom. The problem with someValuesFrom is it says both too much and too little.

gistCore.ttl
a owl:DatatypeProperty ;
rdfs:subPropertyOf gist:endAtDateTime ;
rdfs:range xsd:dateTime ;
skos:definition """This is a date in the past when something actually completed. Might be a project or an historical event.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we clean up this language a bit?

gistCore.ttl
rdfs:range xsd:dateTime ;
skos:definition """This is a date in the past when something actually completed. Might be a project or an historical event.

AtDate refers to a calendar date (birthdays and invoice dates) and is assumed to be precise +/- 12 hours (time zone offset is allowed)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below - clean up annotations.

gistCore.ttl
@@ -2941,6 +2921,20 @@ gist:conversionOffset
skos:prefLabel "conversion offset"^^xsd:string ;
.

gist:deathDate
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as birthdate comment.

gistCore.ttl
gist:hasGiver
a owl:ObjectProperty ;
rdfs:subPropertyOf gist:hasParticipant ;
owl:propertyDisjointWith gist:hasRecipient ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just an unrelated fix? I recommend submitting a separate issue. You could add to the issue that the definitions are very confusing because they don't distinguish hasGiver/hasRecipient from comesFromAgent/goesToAgent.

gistCore.ttl
@@ -3472,7 +3382,6 @@ gist:isAbout

gist:isAffectedBy
a owl:ObjectProperty ;
owl:inverseOf gist:affects ;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these inverses being deleted? And if so, they should be in a separate PR.

gistCore.ttl
gist:tagText
a owl:DatatypeProperty ;
skos:definition "Used for folksonomy style categories (non controlled vocabulary)"^^xsd:string ;
skos:prefLabel "tag text"^^xsd:string ;
.

gist:timeZoneStandardUsed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

usesTimeZoneStandard?

@mkumba
Copy link
Contributor

mkumba commented Jan 26, 2022 via email

@rjyounes
Copy link
Collaborator

rjyounes commented Jan 26, 2022 via email

@mkumba
Copy link
Contributor

mkumba commented Jan 26, 2022 via email

@rjyounes
Copy link
Collaborator

Superceded by #619.

@rjyounes rjyounes closed this Jan 28, 2022
@rjyounes rjyounes deleted the issue-499-cleanups branch February 9, 2022 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjyounes rjyounes rjyounes requested changes

@mkumba mkumba Awaiting requested review from mkumba

@uscholdm uscholdm Awaiting requested review from uscholdm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@marksem @mkumba @rjyounes @uscholdm