search

biolink / ontobio

python library for working with ontologies and ontology associations
https://ontobio.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
119 stars 30 forks source link

Param to set modelstate for geneontology/gocamgen#91 #571

Closed dustine32 closed 3 years ago

dustine32 commented 3 years ago

For geneontology/gocamgen#91.

To use, just need to set --modelstate or -s to production. Default is development.

dustine32 commented 3 years ago

BTW I tested this by loading a few test models to local Noctua and checking the modelstate. Pretty straight-forward. But for unit tests on this change, this is another case where we'd need to generate a model and then check the attribute (i.e. modelstate). Since model generation requires the loaded ontology, which can take 10-30 sec to load, I really need to make a stripped-down, "targeted" ontology that loads super-quick and is small enough to store it with the repo. I may make a ticket just on this problem.

kltm commented 3 years ago

@dustine32 Looks good, but I had a question. Looking at your code, it seems that the default state is only assigned in the cam writer, with it being None elsewhere? Might it be an idea to have that default be development higher up in the code so that other future writers and subsystems would have access?

dustine32 commented 3 years ago

@kltm Yeah, I was thinking of moving up where that default is defined. Probably one level up in GoCamModel? I suppose GoCamModel is the lowest class where someone would care what/if modelstate is emitted to the model.

I can also consolidate how modelstate is passed to GoCamBuilder so that it's not passed through so many functions.

kltm commented 3 years ago

@dustine32 Whatever you think is sensible, I wanted to see where your thoughts are. I think that you're right that the go-cam model would be the place where it starts mattering. I'm just hoping to cut off future spooky issues.

dustine32 commented 3 years ago

@kltm Thanks! You at least got me to reduce the modelstate path, so it should be a little easier to track things down if it does get spooky.

kltm commented 3 years ago

Sounds goo. Feel free to merge when you want.