Update HPO term parsing for new OBO release format

[EddieLF]

Fix for HPO.obo file parsing causing the update-hpo-terms Cloudrun jobs to fail

Based on the changes drafted for new seqr here: https://github.com/populationgenomics/cardinal-seqr/pull/10

Parsing OBO trailing modifier blocks in HPO terms

Problem

New HPO releases now emit lines like:

is_a: HP:0032162 {xref="PMID:31677808"}

The parser strips OBO's ! human-readable label suffix, but not the OBO trailing annotation block {xref="..."}.
(or in combination: is_a: HP:0032162 ! Childhood onset {xref="PMID:31677808"})

After .split(' ! ')[0] we get the literal string HP:0032162 {xref="PMID:31677808"} and store that as parent_id. Later, _get_category_id walks the parent chain and looks up that string in parent_id_map. It's not a real key (only the clean HP:0032162 is), so line 236 raises:

ValueError('Strange id: %s' % hpo_id)

Fix

In get_file_iterator, strip the trailing {...} annotation block in addition to ! label.

• Regex-tidy: re.sub(r'\s*\{[^}]*\}\s*$', '', value.split(' ! ')[0]).strip()

Also add a regression test in update_hpo_tests.py using fixture lines with {xref=...} trailers (both alone and combined with ! label).

The OBO 1.4 spec allows trailing modifiers on basically any clause that takes a target ID (is_a, intersection_of, union_of, relationship, etc.). We currently only parse is_a, so that's the only line to fix.

Testing

>>> import re

>>> # Test with expected ' ! ' and unexpected suffix term '{xref="..."}'
>>> line = 'is_a: HP:0000118 ! Phenotypic abnormality {xref="PMID:31677808"}\n'
>>> value = " ".join(line.split(" ")[1:])
>>> is_a = re.sub(r'\s*\{[^}]*\}\s*$', '', value).split(' ! ')[0].strip()
>>> print(is_a)
HP:0000118

>>> # Test with just unexpected suffix term '{xref="..."}'
>>> line = 'is_a: HP:0000118 {xref="PMID:31677808"}\n'
>>> value = " ".join(line.split(" ")[1:])
>>> is_a = re.sub(r'\s*\{[^}]*\}\s*$', '', value).split(' ! ')[0].strip()
>>> print(is_a)
HP:0000118

[MattWellie]

I think the regression test file is missing, can you add that in?

This will work for this additional failure mode, and matches the current Seqr code (remove/split on substring, what's left should be a HPO term), but I wonder if it's more future proof to switch the logic to a detection which would automatically adjust for any future format changes.

hpo = re.find(r'HP:\d{7}', value)

[EddieLF]

@MattWellie I've added more test coverage for this new change. As for using better regex, we could implement that here, or we could make it part of the other changes in the linked PR from the cardinal-seqr repo

[MattWellie]

Nah I'm sure this is fine, I don't expect the format to change that frequently.

Also I think you need to change a line in the unit test

call('Deleting HumanPhenotypeOntology table with 12 records and creating new table with 7 records'),

(instead of 5 records)

[MattWellie]

LGTMatt