Add seven SPARQL integrity checks and fix what they found by hoodcm · Pull Request #9 · RSNA/RadLex

hoodcm · 2026-09-22T17:51:26Z

Part of #5.

I added seven checks and widened the whitespace check. A class is obsolete when it is filed anywhere under RID15849, as we agreed in #13.

Check	Data checked	Why
No stray spaces	Labels, synonyms, UMLS terms	A search for the name misses it.
No empty values	All annotations	An empty value looks filled in.
RadLex ID matches the IRI	`RadLexID`	A lookup by ID returns the wrong class.
Replacement is a link	`Replaced_by`	Software can't follow a replacement written as text.
Replacement exists	`Replaced_by`	A pointer to a missing class is a typo.
Only obsolete classes have a replacement	`Replaced_by`	A live class has no replacement.
Only obsolete classes have an obsolete name	`Preferred_Name_for_Obsolete`	A live class has no obsolete name.
A class under an obsolete class is marked obsolete	`subClassOf`	An editor who retires a parent can retire its children by accident.

I fixed what the checks found on main. That was 7 labels, 2 ID typos, 4 replacements stored as text, 2 obsolete classes filed in the wrong place, and a stray line in .gitignore. All tests pass.

Widen the whitespace check to rdfs:label and rid:UMLS_Term. Add checks for empty annotation values, rid:RadLexID matching the IRI, class IRIs in RID<digits> form (WARN), and rid:Replaced_by being an IRI, aimed at a live class, and carried only by retired classes. Fix the violations on main: 7 labels, 2 rid:RadLexID typos, 4 rid:Replaced_by literals, 2 chained retirements. Remove a stray conflict marker from .gitignore. Part of #5. Related to #8.

jclerman · 2026-09-28T04:44:40Z

 #
+# The constrained properties are checked directly and through any declared sub-property. Matching only through
+# rdfs:subPropertyOf would skip rdfs:label and rid:UMLS_Term, which have no sub-properties (rid:Synonym is caught
+# that way only because it is declared as a sub-property of itself).


Good catch. rdfs:subPropertyOf is reflexive, so with RDFS entailments, the '*' would not be necessary - but ROBOT does not compute those entailments when it runs these testing queries.

jclerman · 2026-09-28T04:49:57Z

In general, I would like the ROBOT testing queries to be named consistently for what they bind, rather than how they happen to be used. The queries themselves bind violations, so (IMO) should be named for the thing we expect them to return - e.g. empty_annotation_value.rq instead of annotation_value_not_empty.rq.

Please make that change for the new queries?

jclerman · 2026-09-28T04:51:57Z

This one seems like a new policy that we should discuss before adding the test. Even the warning is noise until we make that decision. I'd prefer to remove this, and add the recommendation as an Issue so we can discuss it there.

Removed. I opened #14 for it.

jclerman · 2026-09-28T04:55:08Z

+# VIOLATIONS will be bindings that have, on ?entity, ?property, and ?value respectively:
+# - The entity carrying the empty assertion.
+# - The annotation property.
+# - The empty value.


nit:

Suggested change

# - The empty value.

# - The empty or whitespace value.

jclerman · 2026-09-28T05:14:13Z

+WHERE {
+    BIND(rid:Replaced_by AS ?property)
+    ?entity ?property ?value .
+    FILTER NOT EXISTS { ?entity rid:Preferred_Name_for_Obsolete ?_name }


I recommend removing this line for clarity; we should separately require that it is only present on subclasses of rid:RID15849.

Done. Every check now treats a class as obsolete only when it is under RID15849. The separate check is live_class_with_preferred_name_for_obsolete. It reported two misfiled obsolete classes, and I moved them.

jclerman · 2026-09-28T05:20:00Z

I'm ambivalent about this one. It implies that chains will always be resolvable to a live class. But what if we retire class A as a duplicate of class B, and then retire class B for a different reason (so it has no "replacement")? Do we need to drop the pointer from A to B? It's not hurting anything AFAICT.

Even if we do have a chain that ends in a "live" class, resolving that chain when/as needed seems totally doable by consumers, and I'm not sure that we should take on the maintenance burden (and loss of historical record) implied by needing to repoint.

Counterpoint might be that this rule catches mistakes where a replacement is already retired when it is asserted.

Agreed. The check now reports only a replacement that doesn't exist, and I restored the two chains.

jclerman · 2026-09-28T05:36:23Z

Reviewing the actual ontology changes, I see several cases where values were changed in nontrivial ways (from what I've seen so far, in ". I think those changes deserve close review or at least reporting - not saying they are wrong, but given how difficult it can be to review OWL changes, listing those changes in an easy-to-digest format seems important.

So far I see such changes in RadLexID assertions, and in Replaced_By assertions. The whitespace fixes are more trivial and easy to assess here in github; the others aren't, IMO.

jclerman · 2026-09-28T05:39:08Z

+    ?entity ?property ?value .
+    FILTER(isIRI(?value))
+    FILTER(?value != rid:RID15849)
+    { ?value rid:Preferred_Name_for_Obsolete ?_pname } UNION { ?value rdfs:subClassOf rid:RID15849 }


Use rdfs:subClassOf+: without RDFS inference this only matches direct children of RID15849, so a retired class nested deeper (and lacking Preferred_Name_for_Obsolete) would be missed. + rather than * so RID15849 itself isn't treated as retired.

Done for the parent. With + on the child, the check never returns a result, because any child of an obsolete class is itself under RID15849. I confirmed this with a test case. The check now reports a child of an obsolete class that has no obsolete name of its own.

jclerman · 2026-09-28T05:39:08Z

+    FILTER(?value != rid:RID15849)
+    { ?value rid:Preferred_Name_for_Obsolete ?_pname } UNION { ?value rdfs:subClassOf rid:RID15849 }
+    FILTER NOT EXISTS { ?entity rid:Preferred_Name_for_Obsolete ?_cname }
+    FILTER NOT EXISTS { ?entity rdfs:subClassOf rid:RID15849 }


Same here: rdfs:subClassOf+.

See line 23.

jclerman · 2026-09-28T05:39:08Z

+    BIND(rid:Replaced_by AS ?property)
+    ?entity ?property ?value .
+    FILTER NOT EXISTS { ?entity rid:Preferred_Name_for_Obsolete ?_name }
+    FILTER NOT EXISTS { ?entity rdfs:subClassOf rid:RID15849 }


Use rdfs:subClassOf+: without RDFS inference this only matches direct children of RID15849, so a retired class nested deeper (and lacking Preferred_Name_for_Obsolete) would be missed. + rather than * so RID15849 itself isn't treated as retired.

jclerman · 2026-09-28T05:39:08Z

+    }
+    UNION
+    {
+        { ?value rid:Preferred_Name_for_Obsolete ?_name } UNION { ?value rdfs:subClassOf rid:RID15849 }


Use rdfs:subClassOf+: without RDFS inference this only matches direct children of RID15849, so a retired class nested deeper (and lacking Preferred_Name_for_Obsolete) would be missed. + rather than * so RID15849 itself isn't treated as retired.

Removed with the change above.

…D15849 alone Rename the queries for the violations they return. Drop the RID<digits> IRI check pending its own issue. Use rdfs:subClassOf+ to find obsolete classes at any depth. Add a check that Preferred_Name_for_Obsolete sits only on obsolete classes, and file its two hits (RID27787, RID49508) under RID15849. Narrow the Replaced_by target check to undeclared targets and restore the two chained pointers.

hoodcm · 2026-10-01T16:38:46Z

Thanks, Jeff. I made all the changes in 7d7f44e.

I renamed each check for what it reports, such as empty_annotation_value. These are the changes beyond whitespace:

Class	Change
RID50643 Canale view	ID typo fixed. It had RID60643.
RID50680 invades collecting system	ID typo fixed. It had RID50580, which belongs to another class.
RID10400, RID13601, RID39534, RID39535	Replacement changed from text to a link, with the same target.
RID27787 tenia choroidea of lateral ventricle	Moved under RID15849. It was already marked obsolete.
RID49508 tumor invasion of prostate capsule	Moved under RID15849. It was already marked obsolete.

I also put back the two replacement chains I had changed.

jclerman reviewed Sep 28, 2026

View reviewed changes

hoodcm mentioned this pull request Oct 1, 2026

Two classes have word IRIs instead of RIDs #14

Open

Conversation

hoodcm commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jclerman commented Sep 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jclerman commented Sep 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hoodcm commented Oct 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hoodcm commented Sep 22, 2026 •

edited

Loading