search

geneontology / noctua-models

This is the data repository for the models created and edited with the Noctua tool stack for GO.
http://noctua.geneontology.org/
Creative Commons Attribution 4.0 International
10 stars 3 forks source link

Issue noctua models 170 zfin import test #182

Closed sierra-moxon closed 3 years ago

sierra-moxon commented 3 years ago

fixes #170 fixes #180 fixes #181

Adding ZFIN models to master.

sierra-moxon commented 3 years ago

@dustine32 - I'll have Sabrina take a look at the deleted models.

dustine32 commented 3 years ago

@sierra-moxon Thanks! Looks like you cleared up the contributor bug.

sierra-moxon commented 3 years ago

@dustine32 - confirmed with Sabrina that she is expecting a handful of deleted models. Do model deletes (triggered via noctua interface) only happen when a PR to master occurs?

dustine32 commented 3 years ago

@sierra-moxon Thanks for confirming, at least it's not 100% weird then! Gonna have to confirm with @kltm. My guess is that a model delete through the Noctua UI will remove the model from the in-memory datastore (in minerva) but not immediately in the noctua-models repo. To trigger the delete from the repo, that "automated commit" process would have to run, which dumps out the in-memory models to files and commits them to master, deleting the models in repo if they didn't exist in-memory at that time.

Sort of a question: It looks like your PR here includes the delete changes in the diff, which seems not right to me? I would expect a PR of only new ZFIN models would not touch any of the models created through the Noctua UI. Does this make sense?

kltm commented 3 years ago

Talking to @sierra-moxon , I think there is still a bit of a mystery here as to how the deletes happened here. AFAIK, there is no mechanism to literally delete files in any system right now, including minerva (although @balhoff may know something I don't). I believe(d) that deletes are just an inactive element at this point and we're still working through what exactly we want "delete" to mean.

balhoff commented 3 years ago

@kltm I noticed that Ben added a deletion mechanism to Minerva. I'm not familiar enough with the situation here to know if it's relevant. There is an arg here skipMarkedDelete used when loading to the triplestore: https://github.com/geneontology/minerva/blob/07a2e5690a6f39264e5ca7db8600ccf8bd8131c4/minerva-core/src/main/java/org/geneontology/minerva/BlazegraphMolecularModelManager.java#L608

It's set to true here: https://github.com/geneontology/minerva/blob/07a2e5690a6f39264e5ca7db8600ccf8bd8131c4/minerva-cli/src/main/java/org/geneontology/minerva/cli/CommandLineInterface.java#L484

kltm commented 3 years ago

@balhoff Okay, that actually sounds like what we're experiencing, so I think we're good--we were curious as to why models marked as "delete" were no longer accessible in some cases. (We still have an issue where a few models got removed from a PR, but I think that is separate.)