Skip to content
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

Closed
kltm opened this issue Mar 11, 2021 · 7 comments

Comments

@kltm
Copy link
Member

kltm commented Mar 11, 2021

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

@kltm
Copy link
Member Author

kltm commented Mar 11, 2021

Noting that as a separate concern, we should also make the graph editor less "crashy" when things go wrong here.

@kltm
Copy link
Member Author

kltm commented Mar 11, 2021

Noting that the main issues that discuss this to date seem to be:
geneontology/minerva#212 and
geneontology/go-shapes#197

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.)

@goodb
Copy link

goodb commented Mar 12, 2021

@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.

@kltm
Copy link
Member Author

kltm commented Mar 12, 2021

@goodb Ah, well, yeah--that's a good clarification :)

I'm going to hollow out this out and repurpose this.

@kltm kltm changed the title Clarify minerva return format for validation Update noctua violation handling to conform to nodes-as-model in ShEx in minerva return format for validation Mar 12, 2021
@kltm
Copy link
Member Author

kltm commented Mar 12, 2021

@vanaukenk @goodb Does somebody have a good test model for this?

@goodb
Copy link

goodb commented Mar 12, 2021

@kltm if you create a new model and run the reasoner/validator before adding a title you get the invalid response.

@vanaukenk
Copy link

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants