biothings / biothings_explorer

TRAPI service for BioThings Explorer
https://explorer.biothings.io
Apache License 2.0
10 stars 11 forks source link

BTE potentially not providing primary_knowledge_source on all edges #627

Closed tokebe closed 1 year ago

tokebe commented 1 year ago

Creating an issue for proper organization of discussion:

I'm not sure that primary_knowledge_source is marked on all Edges. These are missing it...

  • If I query CTD through BTE (SmallMolecule MESH:C006303 -> Gene), I see that BTE isn't assigning CTD as the primary source. This is happening for all instances + my local instance (all main branches). This is related to this PR as well
  • this is happening with Multiomics Wellness and Multiomics EHR Risk too: BTE isn't assigning it as primary source (the PR was previously tested and working before...)

_Originally posted by @colleenXu in https://github.com/biothings/biothings_explorer/issues/549#issuecomment-1512440646_

tokebe commented 1 year ago

@colleenXu I'm not sure if I'm generating the query quite correctly as I'm unable to replicate this exactly..Testing on my local, all edges have a primary knowledge source, though many of them are infores:pfocr and there's also an infores:dgidb. Posting the same query to the ARS though the ARAX UI, I get a validation error that one edge has been marked as having primary knowledge sourcenull, which is obviously a problem.

Here's my test PK: 332b694c-4119-4a37-9585-d7fc0774563f

Note that the code to compare primary source from our API_LIST currently depends on the API_LIST name matching the operation api_name, which, given the occasional mismatch, is not the most reliable solution. I'm writing a PR now to change this over to smartapi ID comparison. We may wish to change this to infores at some point, given that that's supported -- we'd just need to add infores to every API in the list.

colleenXu commented 1 year ago

@tokebe I was noticing this behavior when querying the api-specific endpoints for APIs marked primarySource: true.

For CTD local: http://localhost:3000/v1/smartapi/0212611d1c670f9107baf00b77f0889a/query or BTE CI: https://bte.ci.transltr.io/v1/smartapi/0212611d1c670f9107baf00b77f0889a/query ``` { "message": { "query_graph": { "edges": { "e01": { "subject": "n0", "object": "n1", "predicates": ["biolink:related_to"] } }, "nodes": { "n0": { "ids": ["MESH:C006303"], "categories": ["biolink:SmallMolecule"] }, "n1": { "categories": ["biolink:Gene"] } } } } } ```
For Multiomics Wellness local: http://localhost:3000/v1/smartapi/02af7d098ab304e80d6f4806c3527027/query or BTE CI: https://bte.ci.transltr.io/v1/smartapi/02af7d098ab304e80d6f4806c3527027/query ``` { "message": { "query_graph": { "nodes": { "n0": { "ids":["CHEBI:78053"], "categories":["biolink:SmallMolecule"] }, "n1": { "categories":["biolink:ClinicalFinding"] } }, "edges": { "e0": { "subject": "n0", "object": "n1" } } } } } ```
For Multiomics EHR Risk local: http://localhost:3000/v1/smartapi/d86a24f6027ffe778f84ba10a7a1861a/query or BTE CI: https://bte.ci.transltr.io/v1/smartapi/d86a24f6027ffe778f84ba10a7a1861a/query ``` { "message": { "query_graph": { "nodes": { "n0": { "ids":["CHEBI:6637"], "categories":["biolink:SmallMolecule"] }, "n1": { "categories":["biolink:Procedure"] } }, "edges": { "e0": { "subject": "n0", "object": "n1", "predicates": ["biolink:associated_with"], "qualifier_constraints": [ { "qualifier_set": [ { "qualifier_type_id": "biolink:object_direction_qualifier", "qualifier_value": "decreased" } ] } ] } } } } } ```
colleenXu commented 1 year ago

Notes from today's group meeting:

Somewhat higher-priority as a bug, but not a fire / the most urgent Issue has linked PRs, showing that code was written and passed testing before. Unclear why it's not working now… JC has already updated the Translator Deadline Tracker

Leaving to JC

colleenXu commented 1 year ago

@tokebe probably can drop this issue because of the migration to TRAPI 1.4?

tokebe commented 1 year ago

Yeah, I'd say we mark this one closed after the prod deployment, and the re-open if it comes up again.

tokebe commented 1 year ago

Closing as TRAPI 1.4 is on Prod.

colleenXu commented 1 year ago

Noting:

So it's correct to close this issue as completed / not really relevant anymore