-
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
Record class doesn't handle qualifier value arrays in associations #713
Comments
@colleenXu, is there an expected behavior when an association has qualifier value arrays, but a returned edge querying that association has no qualifiers? This particular case isn't represented in the example query, so this is unfortunately a purely hypothetical question at the moment. |
Err....it sounds like there's two issues happening? 1: somehow there are records/associations where a qualifier's value is an array rather than a string
2: It's not clear to me what "association has qualifier value arrays, but a returned edge querying that association has no qualifiers" means. Is it one of these situations?
|
I should note that by "association", I mean the |
So, what I mean is, the smartapi-kg package gives us an operation, the To give more insight, to my memory, there's no specific reasoning why the Record class should fall back to the association-defined qualifiers if the actual returned edge (from which we're building the Record) has no As for where we're seeing an actual error (the "When reversing a record made from such an association" part), that's just a detail of Record class reversing which is totally understood and just needs some fixing, no further discussion is needed. |
So....it sounds like none of the scenarios I described above are really what's going on? So...I'm going to try again at understanding the problem and asking questions: It now sounds like 1
And can you say what KPs these
On "going back to the association because qualifiers aren't in the record": that's why I'm trying to figure out if this is a TRAPI KP issue or not.
And it sounds like the core problem IS NOT "KP response records having no qualifiers when we expected them" (purely hypothetical; I guess it's okay that I still don't quite get this) or "reversing for records"... |
Relates to this PR which affects internal metaKG parsing. It looks like this only affects TRAPI KPs, yes.
Yes, seemingly only for TRAPI KPs based on the code/PRs I'm looking at.
That was my understanding as well...one operation, one possible qualifier set.
An
I need to isolate which KP is associated with our actual error to try and pinpoint what it looks like when this
I think this fallback code is specifically there for non-TRAPI KPs, where we insert those qualifiers because, as you said, they're not in what we're getting back from the KP. In which case, we probably won't ever see this code triggered by a TRAPI KP failing to provide qualifiers, so I suppose I don't really need to handle that case.
What remains are two things that I have to do:
@rjawesome, though I understand you're busy, if you get a free moment and happen to recall anything that could give us insight into that second point, it'd be greatly appreciated. |
Basically, in the meta_knowledge_graph response for any TRAPI API contains an array of qualifiers. Each qualifier has the name of the qualifier and applicable values (plural). This means that the API could accept multiple values for that qualifier name, so we would need to store all of the values in the association. I suppose if we didn't want to allow arrays then we would have to modify the code from the PR so that it splits into multiple associations for each of the applicable values for a given qualifier. example from bte meta_knowledge_graph (currently an entry like this would be processed as one association): "subject": "biolink:SmallMolecule",
"predicate": "biolink:associated_with",
"object": "biolink:PhenotypicFeature",
"qualifiers": [
{
"qualifier_type_id": "biolink:object_aspect_qualifier",
"applicable_values": [
"likelihood"
]
},
{
"qualifier_type_id": "biolink:object_direction_qualifier",
"applicable_values": [
"increased",
"decreased"
]
}
],
"knowledge_types": [
"lookup"
] |
Thank you for the clarity @rjawesome, this makes perfect sense! @colleenXu the current code works (aside from the record reversing issue) to handle this just fine, but I'm of two minds whether it should instead be changed to make one association = one qualifier value. What do you think? |
Finding some specific examples from TRAPI KPs /meta_knowledge_graph responses: Automat CTD - a LOT!
https://automat.test.transltr.io/ctd/1.4/meta_knowledge_graph Hmmm...so these edges aren't inverses of each other?
Automat Pharos
https://automat.test.transltr.io/pharos/1.4/meta_knowledge_graph
Automat Hetio
However, many of these seem outside the chem-gene/gene-gene scope of qualifiers right now or don't quite seem to make sense https://automat.test.transltr.io/hetio/1.4/meta_knowledge_graph
Automat drug-central
https://automat.test.transltr.io/drugcentral/1.4/meta_knowledge_graph
Automat gtex
https://automat.test.transltr.io/gtex/1.4/meta_knowledge_graph
Automat gtopdb
https://automat.test.transltr.io/gtopdb/1.4/meta_knowledge_graph
|
From meetings: Moving forward with changing code to ensure only one value per internal association object. |
From what I recall (@tokebe, please comment with agreement or clarification):
A complication to building these "qualifier-sets" is what to do about the potential absence of qualifiers or specific type-ids. For example, what if a TRAPI KP:
|
@tokebe Linked to this PR biothings/api-respone-transform.js#60? Just in case, I put both this issue and the PR into the Project manager/deployment section... |
Relevant changes deployed to Prod. |
In some cases, the association object for an edge will have qualifiers with multiple values in an array. When reversing a Record made from such an association, or when falling back to the association in cases where a returned edge is missing those qualifiers, the Record will TypeError as it tries to do string operations on the array.
Similarly, because an association can have qualifier value arrays, falling back to the association qualifiers when the returned edge is missing a qualifier can cause information problems, such as when the array has both
increased
anddecreased
.We need specific expectations for how to handle the fallback, and the code should check for
string
orstring[]
before operating on qualifier values.Reproducible with the following query:
The text was updated successfully, but these errors were encountered: