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

3 terms missing rdfs:isDefinedBy triples #775

Closed
pmcb55 opened this issue Nov 21, 2022 · 7 comments · Fixed by #927
Closed

3 terms missing rdfs:isDefinedBy triples #775

pmcb55 opened this issue Nov 21, 2022 · 7 comments · Fixed by #927
Assignees
Labels
impact: patch No new functionality or changes in human-readable semantics (e.g,. fixing a typo in an annotation)

Comments

@pmcb55
Copy link

pmcb55 commented Nov 21, 2022

I'm writing some basic Best Practice-checking code, and in the process I noticed that there are just 3 terms (from the 289 terms in gist) that are missing rdfs:isDefinedBy triples, namely:

  • gist:_day
  • gist:_millisecond
  • gist:_minute

I assume this is just a minor oversight (as the other 14 owl:NamedIndividual instances do include that triple). And it's easy to fix up by just adding the missing triples.

@rjyounes
Copy link
Collaborator

This is strange. rdfs:isDefinedBy assertions are auto-generated by our release packaging tool ontology-toolkit. I can see no reason why it would apply to some of these individuals and not others. We'll need to explore.

@dylan-sa
Copy link
Contributor

dylan-sa commented Dec 8, 2022

I looked into this and found the issue. When generating the rdfs:isDefinedBy assertions, onto-tool has two settings--a default 'strict' setting and an 'all' setting. With the strict mode enabled, it only adds the assertions for entities that belong to certain classes (including owl:Thing). These three duration units are missing explicit owl:Thing type assertions, which caused them to be filtered out. Adding the missing owl:Thing assertions will resolve the issue for future releases.

@Jamie-SA
Copy link
Contributor

In the discussion for PR #783 it looks like instead of adding owl:Thing assertions, @rjyounes suggested changing the onto_tool config to address this issue.

Modify our bundle.yaml file to use all rather than strict. Are there any negative consequences to doing so?

@Jamie-SA Jamie-SA self-assigned this Jan 26, 2023
@stevenchalem
Copy link
Contributor

@Jamie-SA @dylan-sa Should we keep this in the Feb. release?

@Jamie-SA
Copy link
Contributor

Jamie-SA commented Feb 8, 2023

This sounded easy... but I can't see a way to get the current version of the onto_tool to use the all mode. The way we are currently adding the definedBy property does not support the all option, and the only action that does is the export which looks like it does other things we don't want.

To get this functionality it looks like a change to the onto_tool would be required.

So I guess maybe we move this to the next release.

@Jamie-SA Jamie-SA added the impact: patch No new functionality or changes in human-readable semantics (e.g,. fixing a typo in an annotation) label Jun 8, 2023
@rjyounes
Copy link
Collaborator

We'll do this as a short-term workaround until onto_tool is fixed.

@Jamie-SA
Copy link
Contributor

Fixed in PR #927 but not closed because this isn't the default branch. So I'm closing it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: patch No new functionality or changes in human-readable semantics (e.g,. fixing a typo in an annotation)
Projects
None yet
5 participants