-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
For Multiomics/Text-Mining APIs, avoid merging records that only differ by their edge.sources
contents
#783
Comments
Fix implemented as part of biothings/api-respone-transform.js#63 |
oh ooops, I didn't realize that the solution for Monarch API actually may work for all APIs? I guess my next step is finding a example to test, to confirm that it is working with another API... |
The fix I implemented in the Monarch PR affects how Record hashes are calculated across the board (i.e. all records now calculate their hash using their source/provenance chain), so if I understand this issue correctly, the fix should cover it completely. |
Jackson and I confirmed that biothings/api-respone-transform.js#63 addresses #775 in a more robust way. So we can revert biothings/bte_trapi_query_graph_handler#178 as part of #784 (changing that issue to be about removing temporary things we no longer need). How I tested this First, in a local install of BTE, be in all main branches. Remove or comment out the line added in https://github.com/biothings/bte_trapi_query_graph_handler/pull/178/files. (remember to build if needed to incorporate the change) Then send this query to Multiomics biggim-drug-response through BTE: http://localhost:3000/v1/smartapi/adf20dd6ff23dfe18e8e012bde686e31/query Query
The response will have an odd edge to the gene KIT (NCBIGene:3815), with two primary knowledge sources in the odd edge: bug
Then check out the branch for biothings/api-respone-transform.js#63 (remember to build if needed to incorporate the change). Then run the same query again. The response will change to two separate edges, 1 for TTD and 1 for drugcentral
|
Also, I think this will only be an issue for Multiomics biggim-drug-response, because that KP uses multiple sources. VS the other APIs are basically single source:
|
@colleenXu Can this issue be closed? Relevant BTE code is now on Prod. |
Confirmed that it's fixed now: there's two edges when posting the example query to |
In both #775 and #774 (comment) (see third bullet in "implementation notes" and the collapsed details), we noticed that BTE will merge records that have different TRAPI-edge-source content. For x-bte annotation, we uses the
trapi_sources
response-mapping keyword (feature from #617) to tell BTE to ingest/handle this content.This can lead to undesired behavior, like a TRAPI edge that has two primary knowledge sources - this should cause a TRAPI validation error. See those issues for examples.
A fix specific for Monarch API was implemented in biothings/api-respone-transform.js@3534b23, but it would be nice to implement a solution for all APIs that use
trapi_sources
. Right now, these are only the BioThings APIs we make in collab with other Translator teams, Multiomics/Text-Mining.Note: If it's possible, it may be nice to have a solution that could theoretically work with any API (non-BioThings, external), just in case we do post-processing to create TRAPI-source-content with other APIs in the future (similar to, but probably more involved, than what we did with Monarch API).
The text was updated successfully, but these errors were encountered: