opentargets / issues

Issue tracker for Open Targets Platform and Open Targets Genetics Portal
https://platform.opentargets.org https://genetics.opentargets.org
Apache License 2.0
12 stars 2 forks source link

Broken EFO link in the disease page #3108

Closed ireneisdoomed closed 8 months ago

ireneisdoomed commented 12 months ago

Describe the bug The EFO link in the header of every disease page was leading to a broken link in EFO because OLS was down.

Observed behaviour The header of a disease page is using a hardcoded link to OLS: https://github.com/opentargets/ot-ui-apps/blob/d0b0b0acc427b3d65ec8835706638716feaa91d5/apps/platform/src/pages/DiseasePage/Header.jsx#L40

Expected behaviour We want to use identifiers.org, to avoid such problems in the future.

Additional context The broken link was happening because OLS is going to be updated from version 3 to 4.

LucaFumis commented 11 months ago

I have updated the hardcoded link to use OLS v4. I couldn't find any official documentation on this, but it appears correct to me (all documentation seems to be about API use). To my understanding, identifiers.org currently only supports EFO (still pointing to old OLS), and not linking directly to OLS. So not sure how to handle for example MONDO ids?

DSuveges commented 11 months ago

@ireneisdoomed

We want to use identifiers.org, to avoid such problems in the future.

I'm not sure about how it would solve the problem: the identifier.org link directly jumps to OLS, so if the OLS is down, having identifiers.org link wouldn't save us. Also EFO contains labels of different namespaces, it seems they won't work with identifiers.org:

ireneisdoomed commented 11 months ago

My bad, I thought identifiers.org supported all of our ontologies. These are all current prefixes in the disease index:

+--------+
|  prefix|
+--------+
|    NCIT|
|Orphanet|
|      GO|
|      HP|
|    OTAR|
|     EFO|
|   MONDO|
|     OBI|
|     OBA|
|    DOID|
|      MP|
|    OGMS|
+--------+

For now the safest solution seems to update the links to use OLS as @LucaFumis has done. identifiers.org is useful because it not only gives support to disease-related ontologies, but other important entities we handle in other components: ENSEMBL, Uniprot, ClinVar, etc. As a long term solution, I think we should request them to add the missing prefixes. We should ask identifiers.org to incorporate these. Apart from OTAR*, all the other ontologies come from EFO terms, so I see no reason why they should exclude them. What do you think @DSuveges?

*OTAR links at the moment point nowhere. There are just 9 of them. I don't think it's a problem, just a note.

DSuveges commented 11 months ago

I also agree, the correct way would be to use the updated OLS URL. I see it as a permanent solution.

Personally, I don't see any value using identifiers.org. Also my view on this issue, is that if we are using EFO as ontology, we totally should link out to EFO all the time, even if the prefix is MONDO, NCIT or something else. Mostly because EFO can provide the correct context in the disease hierarchy (most of the time).

Let's take Mitochondrial disease with peripheral neuropathy, the ontology link, in my understanding should point to EFO (as it is now), and not Orhanet as if the identifiers.org id is used: https://identifiers.org/orphanet:225703. Of course we can still have link to orphanet.

*OTAR links at the moment point nowhere

To me it seems there's no link to any ontology. There's a slight discrepancy between the EFO exposed on OLS and "our" version of EFO, which is not exactly the same.

LucaFumis commented 11 months ago

Thank you @DSuveges and @ireneisdoomed for your feedback. We'll review and merge the proposed fix (OLS URL).

LucaFumis commented 11 months ago

Re-assigning to @carcruz for the @opentargets/fe-team

No action required: PR approved and merged. Issue can probably be closed.