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.

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

Issue 541: Add Event #542

Merged
merged 4 commits into from
Nov 13, 2023
Merged

Issue 541: Add Event #542

merged 4 commits into from
Nov 13, 2023

Conversation

sbarnum
Copy link
Contributor

@sbarnum sbarnum commented Jul 19, 2023 •
edited by ajnelson-nist
Loading

This Pull Request resolves all requirements of Issue #541 .

Coordination

  • Pull Request is against correct branch
  • Pull Request is in, or reverted to, Draft status before Solutions Approval vote has passed
  • CI passes in UCO feature branch against develop
  • CI passes in UCO current unstable branch (fd8e56c)
  • CI passes in CASE current unstable branch tracking UCO's unstable as submodule (7adb16d)
  • Impact on SHACL validation reviewed for CASE-Examples
  • Impact on SHACL validation remediated for CASE-Examples (N/A)
  • Impact on SHACL validation reviewed for casework.github.io
  • Impact on SHACL validation remediated for casework.github.io (N/A)
  • Milestone linked
  • Solutions Approval vote logged on corresponding Issue (once logged, can be taken out of Draft PR status)

@sbarnum
Added classes, properties and shapes for Event
f717616 
Added new core:Event class with appropriate SHACL shapes
Added new core:eventType property
Added new core:eventContext property
Added new core:eventAttribute property
@sbarnum sbarnum mentioned this pull request Jul 19, 2023
Closed
14 tasks
@ajnelson-nist
Copy link
Contributor

This should have targeted develop; I'm editing the PR now to re-target.

@ajnelson-nist ajnelson-nist changed the base branch from master to develop July 19, 2023 18:44
@ajnelson-nist ajnelson-nist added this to the UCO 1.x.0 milestone Jul 19, 2023
@ajnelson-nist ajnelson-nist marked this pull request as draft July 19, 2023 19:42
@ajnelson-nist
Copy link
Contributor

@sbarnum , FYI - To prevent premature merges, PRs are left in Draft form until the passing Solutions Approval vote is logged.

plbt5
plbt5 previously requested changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@plbt5 plbt5 left a comment

Choose a reason for hiding this comment

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

I interpret an Event as something that expands and/or develops over time, a Perdurant. Furthermore, in the definition of the UcoObject I cannot find any reference to something that is not an Endurant.

In this interpretation we do realize, don't we, that by subclassing the Event from the UcoObject we cannot make a distinction anymore between Endurants and Perdurants, let alone make them disjoint?! In my professional opinion, such distinction is a necessary feature for an interoperability standard to provide.

As the ontologist in CDO, I don't approve of this design.

A simple resolution would be to subclass the Event from the UcoThing, like the UcoObject and UcoInherentCharacterizationThing, and make those three disjoint from each other.

@ajnelson-nist ajnelson-nist linked an issue Jul 19, 2023 that may be closed by this pull request
Need ability to represent general concept of an Event #541
Closed
14 tasks
@ajnelson-nist ajnelson-nist changed the title Feature issue 541 Issue 541: Add Event Jul 19, 2023
@sbarnum
Copy link
Contributor Author

sbarnum commented Jul 19, 2023

This should have targeted develop; I'm editing the PR now to re-target.

Hmm. I definitely chose 'develop' as the target as I had to change it from the default of 'master'.
Not sure what happened. Thanks for fixing.

@sbarnum
Copy link
Contributor Author

sbarnum commented Jul 19, 2023

I interpret an Event as something that expands and/or develops over time, a Perdurant. Furthermore, in the definition of the UcoObject I cannot find any reference to something that is not an Endurant.

In this interpretation we do realize, don't we, that by subclassing the Event from the UcoObject we cannot make a distinction anymore between Endurants and Perdurants, let alone make them disjoint?! In my professional opinion, such distinction is a necessary feature for an interoperability standard to provide.

As the ontologist in CDO, I don't approve of this design.

A simple resolution would be to subclass the Event from the UcoThing, like the UcoObject and UcoInherentCharacterizationThing, and make those three disjoint from each other.

To honor Alex's request to keep this level of discussion on the issue rather than the PR I will wait until Paul moves his comments over to the issue before responding there.

@plbt5
Copy link
Contributor

plbt5 commented Jul 20, 2023

To honor Alex's request to keep this level of discussion on the issue rather than the PR I will wait until Paul moves his comments over to the issue before responding there.

Which I did, with appropriate apologies 😊

ajnelson-nist
ajnelson-nist requested changes Jul 20, 2023
View reviewed changes
ontology/uco/core/core.ttl Outdated Show resolved Hide resolved
ontology/uco/core/core.ttl Show resolved Hide resolved
@ajnelson-nist
Fix datatype constraints
36ee236 
Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
@ajnelson-nist
Add stub reference to types:Dictionary
9cf555a 
References:
* #541

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
@ajnelson-nist ajnelson-nist modified the milestones: UCO 1.x.0, UCO 1.3.0 Aug 29, 2023
@ajnelson-nist ajnelson-nist marked this pull request as ready for review November 13, 2023 19:39
ajnelson-nist
ajnelson-nist approved these changes Nov 13, 2023
View reviewed changes
Copy link
Contributor

@ajnelson-nist ajnelson-nist left a comment

Choose a reason for hiding this comment

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

My review remarks have been addressed.

@ajnelson-nist
Copy link
Contributor

@plbt5 , I am "Dismissing"---using the GitHub interface terminology---your blocking review on this PR, because:

So, for the sake of this PR, I believe your remarks have been sufficiently addressed. I'm merging the PR now in acknowledgement of the Solutions Approval vote having been logged.

@ajnelson-nist ajnelson-nist dismissed plbt5’s stale review November 13, 2023 19:48

Review matters are addressed on corresponding Issue and Issue 544.

@ajnelson-nist ajnelson-nist merged commit 9727fe3 into develop Nov 13, 2023
1 check passed
@ajnelson-nist ajnelson-nist deleted the Feature-Issue-541 branch November 13, 2023 19:48
ajnelson-nist added a commit that referenced this pull request Nov 20, 2023
@ajnelson-nist
Designate action:Action and core:Event disjoint
77cd47d 
In an OWL-only sense, the ontology portion of this patch could have been
accomplished by adding one triple to `action:Action`.

For the sake of symmetry and explicitness, `core:Event` also picked up
the `owl:disjointWith` statement.  To satisfy OWL syntactic
requirements, a stub reference to `action:Action` was added, as was done
for `types:Dictionary` for Issue 541 (discussed in PR 542).

To enforce disjointedness with SHACL, a new independent shape is added,
`action:Action-disjointWith-Event-shape`.  The shape is spelled using an
anonymous node shape to avoid a multiple-inheritance issue:

* SHACL syntactically requires the subject of a triple with `sh:not` as
  predicate be a blank node.
* Separately, SHACL also only permits a shape to have up to one triple
  with `sh:not` as predicate.

Hence, if the `action.ttl` file were included twice in some graph,
SHACL-SHACL validation would fail, because the identically-written blank
nodes that are `sh:not` objects would not resolve to the same node.
Using an anonymous wrapping `sh:NodeShape` avoids this issue.
(Multiple inheritance would now cause some redundant processing to occur,
but SHACL-SHACL validation would pass.)

A follow-on patch will regenerate Make-managed files.

References:
* #563
* #542
* https://www.w3.org/TR/shacl/#shacl-shacl

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
ajnelson-nist added a commit that referenced this pull request Nov 22, 2023
@ajnelson-nist
Designate action:Action and core:Event disjoint
6669aab 
In an OWL-only sense, the ontology portion of this patch could have been
accomplished by adding one triple to `action:Action`.

For the sake of symmetry and explicitness, `core:Event` also picked up
the `owl:disjointWith` statement.  To satisfy OWL syntactic
requirements, a stub reference to `action:Action` is added, as was done
for `types:Dictionary` for Issue 541 (discussed in PR 542).

To enforce disjointedness with SHACL, a new independent shape is added,
`action:Action-disjointWith-Event-shape`.

A follow-on patch will regenerate Make-managed files.

EDIT 2023-11-22: An initial version of this patch included an inlined
anonymous `sh:NodeShape`.  I now believe the scenario that shape
mitigates (multi-import of a shapes graph leading to SHACL-scoped syntax
errors) is unlikely, and I also saw that the noted rationale around
`sh:not` was confused with another predicate (`sh:inversePath`).

References:
* #563
* #542

Signed-off-by: Alex Nelson <alexander.nelson@nist.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajnelson-nist ajnelson-nist ajnelson-nist approved these changes

@plbt5 plbt5 plbt5 left review comments

Assignees
No one assigned
Labels
Change Proposal
Projects
None yet
Milestone
UCO 1.3.0
Development

Successfully merging this pull request may close these issues.

Need ability to represent general concept of an Event
3 participants
@sbarnum @ajnelson-nist @plbt5