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

Auto PR: assertion_evidence.yaml generated from google spreadsheets using schemasheets #123

Closed
wants to merge 2 commits into from

Conversation

puja-trivedi
Copy link
Collaborator

This PR adds a autogenerated assertion_evidence.yaml file.

@djarecka
Copy link
Collaborator

looking at the ER diagram, and there are couple connections that are not clear to me:

  • there are two connections between Agent and Activity. I think we need at most one connection, and the part Agent -> was_associated_with-> Activity should be removed
  • I asked also yesterday, but I'm not sure what is EvidenceSummary that is connected to EvidenceLine
  • Also shouldn't we have EvidenceItem connected to EvidenceLine instead of just Document, as it is in sepio
  • There are boxes that are not connected to anything, I'm not sure if we need right now classes for ManualAssertion and AutomaticAssertion, maybe we can just have this as possible values for AssertionType for now?
  • There is also Evidence that should be removed?

@tekrajchhetri -please comment

@patrick-lloyd-ray
Copy link
Member

there are two connections between Agent and Activity. I think we need at most one connection, and the part Agent -> was_associated_with-> Activity should be removed

IMO, agent participates_in activity is the appropriate relation

@tekrajchhetri
Copy link
Contributor

@djarecka Please find my answer below.

  • there are two connections between Agent and Activity. I think we need at most one connection, and the part Agent -> was_associated_with-> Activity should be removed.
 I agree with @patrick-lloyd-ray suggestion for `Agent` and `Activity` relation.
  • I asked also yesterday, but I'm not sure what is EvidenceSummary that is connected to EvidenceLine

The EvidenceSummary is like a justification or some author's description. Now that we're using the Annotation class, we should remove it and connect it to Annotation.

  • Also shouldn't we have EvidenceItem connected to EvidenceLine instead of just Document, as it is in sepio

We can include the EvidenceItem. However, we should be fine without it as well. If you see the image you can notice that EvidenceItem is again described by Document.

  • There are boxes that are not connected to anything, I'm not sure if we need right now classes for ManualAssertion and AutomaticAssertion, maybe we can just have this as possible values for AssertionType for now?

AssertionType are subclass of AssertionMethod and it seems that ER-diagram generator doesn't visualize it now, something that I was discussing in the SLACK. However, when I generated the OWL file, the subclass-hierarchy are generated correctly.

  • There is also Evidence that should be removed?

Yes, we agreed to replace Evidence with EvidenceItem. Probably, @puja-trivedi missed it?

@tekrajchhetri
Copy link
Contributor

Update: I checked the file and I can see the classes EvidenceLine and Annotation used. Not sure why it has not been reflected in ER-diagram. Maybe there's issue with the generator? @puja-trivedi ?

@@ -85,6 +123,12 @@
"title": "AssertionType",
"type": "string"
},
"AutomaticAssertion": {
"additionalProperties": false,
"description": "An assertion method that does not involve human review.",
Copy link
Member

Choose a reason for hiding this comment

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

Generally speaking, but with some exceptions, I would like to avoid descriptions that classify via negation. Could this be: "An assertion method that is made via an automated process."? Perhaps if this is intended only to cover in silico assertions, we could add a clause to that effect.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have updated the google sheet with An assertion method executed automatically by machines without human intervention. definition. So next time when we generate the model, it should replace this negated definition.

@puja-trivedi
Copy link
Collaborator Author

there are two connections between Agent and Activity. I think we need at most one connection, and the part Agent -> was_associated_with-> Activity should be removed

IMO, agent participates_in activity is the appropriate relation

@patrick-lloyd-ray - we don't see participates_in in the prov ontology. Do you think we should define this attribute ourselves or is this from another ontology?

@patrick-lloyd-ray
Copy link
Member

there are two connections between Agent and Activity. I think we need at most one connection, and the part Agent -> was_associated_with-> Activity should be removed

IMO, agent participates_in activity is the appropriate relation

@patrick-lloyd-ray - we don't see participates_in in the prov ontology. Do you think we should define this attribute ourselves or is this from another ontology?

Oh, I was thinking in BFO/RO world. According to the RO->PROV mapping, it should be was associated with.

@djarecka
Copy link
Collaborator

djarecka commented Jan 6, 2025

worked moved to #130

@djarecka djarecka closed this Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants