-
Notifications
You must be signed in to change notification settings - Fork 13
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
Update noctua violation handling to conform to nodes-as-model in ShEx in minerva return format for validation #714
Comments
Noting that as a separate concern, we should also make the graph editor less "crashy" when things go wrong here. |
Noting that the main issues that discuss this to date seem to be: As well. the test used https://github.com/berkeleybop/bbop-graph-noctua/blob/cc126170af38f0e962de56d3f9e1b865cab3757a/tests/response-gomodel-5d88482400000052-2019-09-25.json Looking through the conversation, I believe that a model for a node is probably a bug. (Again, disregarding that we can make the client less crashy.) |
@kltm if it helps, the shex validator does intentionally use the model identifier as a node when checking and when reporting about validation of the model-level metadata (e.g. title). It seems maybe there is conflict with the concept of "node" in shex sense that allows the identifier of the model itself to be what it considers a node and the concept of a node in the graph client which likely does not. |
@goodb Ah, well, yeah--that's a good clarification :) I'm going to hollow out this out and repurpose this. |
@vanaukenk @goodb Does somebody have a good test model for this? |
@kltm if you create a new model and run the reasoner/validator before adding a title you get the invalid response. |
@kltm - I tested running the reasoner in the graph editor on a model that doesn't have a title. The reasoner runs fine and the background on the graph editor turns green (there were otherwise no logical errors), but the Valid status on the model is 'INVALID' . Adding a title to the model sets its Valid status to 'true'. This looks like the expected behavior, although future work could be done to feedback about needing a model title. I'm closing this ticket for now. Please feel free to re-open, if need be. |
Looking at some of the recent discussions around violation checking (reasoner/shex), I think it would be good to go over the expectations around the responses from minerva.
As pointed out here:
geneontology/minerva#371 (comment)
minerva is returning model identifiers for
node
, which I do not believe was part of the original spec. (I'm still looking for where Ben and myself hashed this out originally.)To move other tickets along, like #711, I think it might be good to come to a conclusion about the schema and appropriate contents are for the violations and then work forward from there.
Tagging @balhoff @tmushayahama
The text was updated successfully, but these errors were encountered: