Skip to content

norm: preserve canonical OBO prefix case in obo_handle (fixes ~503k dangling edges in monarch-kg)#40

Open
kevinschaper wants to merge 1 commit into
Knowledge-Graph-Hub:mainfrom
kevinschaper:fix/obo-handle-preserve-prefix-case
Open

norm: preserve canonical OBO prefix case in obo_handle (fixes ~503k dangling edges in monarch-kg)#40
kevinschaper wants to merge 1 commit into
Knowledge-Graph-Hub:mainfrom
kevinschaper:fix/obo-handle-preserve-prefix-case

Conversation

@kevinschaper

Copy link
Copy Markdown
Contributor

Summary

universalizer/norm.py::obo_handle converts OBO:<prefix>_<id> CURIEs into <prefix>:<id> form. The final step .upper()s the prefix portion:

# Capitalize the prefix, but not the rest of the ID
split_id = new_id.split(":", 1)
new_id = f"{split_id[0].upper()}:{split_id[1]}"

The comment describes the intent ("Capitalize"), but .upper() is the wrong operation. It's a no-op for OBO namespaces whose canonical CURIE is all-uppercase (HP, MONDO, UBERON, CL, ...) but it silently breaks any mixed-case OBO prefix.

What this caused in the wild

phenio.json has 372k canonical http://purl.obolibrary.org/obo/FBbt_NNNNNNNN URIs (and 44k WBbt_ URIs). KGX's default JSON-LD context doesn't have explicit entries for FBbt or WBbt, so it falls back to the generic OBO: prefix and writes OBO:FBbt_NNNNNNNN. universalizer then .upper()-ed the prefix, producing FBBT:NNNNNNNN — which is not the canonical OBO Foundry CURIE for fly anatomy.

Downstream consumers (e.g. monarch-kg's Alliance gene-to-expression ingest) look up nodes by the canonical mixed-case form, so every Drosophila + C. elegans gene-expression edge went to the dangling-edge pile in every monarch-kg release — about 503,187 edges per release (alliance_gene_to_expression_edges: 397,914 FBbt + 105,273 WBbt, ~99.7% of that ingest's anatomy-side danglers).

Fix

Drop the .upper() and preserve the case that came in from the OBO PURL local-name segment. Behaviour for all-uppercase OBO prefixes is unchanged (.upper() was a no-op for them); behaviour for mixed-case prefixes (FBbt, WBbt, ZFA, XAO, ...) is now correct.

-        # Capitalize the prefix, but not the rest
-        # of the ID
-        split_id = new_id.split(":", 1)
-        new_id = f"{split_id[0].upper()}:{split_id[1]}"
+        # Preserve the canonical case of the prefix as it appears in the
+        # source OBO PURL — `FBbt`, `WBbt` etc. stay mixed-case; pure
+        # uppercase prefixes (HP, MONDO, UBERON, CL, ...) are unchanged.

Tests added

A TestObohandle class with:

  • test_uppercase_prefix_unchanged: HP / MONDO / UBERON / CL still produce the expected uppercase CURIEs (regression guard).
  • test_mixed_case_prefix_preserved: FBbt / WBbt come out canonical.

Both pass.

Verification (end-to-end through clean_and_normalize_graph)

before patch:
  OBO:FBbt_00000001 → FBBT:00000001  ✗
  OBO:WBbt_0000001  → WBBT:0000001   ✗

after patch:
  OBO:FBbt_00000001 → FBbt:00000001  ✓
  OBO:WBbt_0000001  → WBbt:0000001   ✓
  OBO:HP_0000118    → HP:0000118     ✓ (unchanged)
  OBO:MONDO_0000001 → MONDO:0000001  ✓ (unchanged)

Related

  • monarch-initiative/monarch-app tracking issue: (will link)
  • Earlier kg-phenio PR (knowledge-graph-hub/kg-phenio#201) was based on a wrong hypothesis (that the prefixmap context was the cause). It merged but the fix had no effect because the .upper() in this file runs regardless of which prefixmap context is passed.
  • Once this lands and a new universalizer is released, kg-phenio + monarch-kg will start producing canonical FBbt: / WBbt: nodes and the 503k dangling edges will resolve automatically — no consumer-side workaround needed.

`obo_handle` strips `OBO:` from CURIEs like `OBO:FBbt_00000001` and
rebuilds them as `FBbt:00000001`. The final step was uppercasing the
prefix portion with `.upper()`:

    new_id = f"{split_id[0].upper()}:{split_id[1]}"

The accompanying comment ("Capitalize the prefix") describes the intent,
but `.upper()` is the wrong operation: it preserves the case for OBO
namespaces that are conventionally all-uppercase (HP, MONDO, UBERON,
CL, ...) but breaks any mixed-case OBO prefix.

In practice this caused every Drosophila and C. elegans gene-expression
edge in monarch-kg to dangle, because:

  - phenio.json source has canonical IRIs like
    http://purl.obolibrary.org/obo/FBbt_NNNNNNNN.
  - KGX (which lacks an explicit prefix entry for FBbt / WBbt in its
    default JSON-LD context) falls back to writing them as
    `OBO:FBbt_NNNNNNNN`.
  - This function then `.upper()`-ed the prefix, producing `FBBT:` and
    `WBBT:` — neither of which is the canonical OBO Foundry CURIE for
    those namespaces (`FBbt:`, `WBbt:`).
  - Downstream consumers like the Alliance gene-to-expression ingest
    look up nodes by the canonical form, so all ~503k Drosophila +
    C. elegans gene-expression edges per monarch-kg release went to
    the dangling-edge pile.

Drop the `.upper()` and preserve the case as it appears in the OBO
PURL local-name segment. Behaviour is unchanged for all-uppercase OBO
prefixes (HP/MONDO/UBERON/CL/...) — the `.upper()` was a no-op for those
already — and now correct for mixed-case prefixes (FBbt/WBbt/ZFA/XAO/...).

Add regression tests asserting both behaviours.

Verified end-to-end on a synthetic input through `clean_and_normalize_graph`:

  before: OBO:FBbt_00000001 → FBBT:00000001
  after:  OBO:FBbt_00000001 → FBbt:00000001
  HP / MONDO / UBERON / CL unchanged.
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.

1 participant