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

Upgrade to OWL API 4.5.29. #327

Merged
merged 1 commit into from
Jun 12, 2024
Merged

Upgrade to OWL API 4.5.29. #327

merged 1 commit into from
Jun 12, 2024

Conversation

balhoff
Copy link
Member

@balhoff balhoff commented Jun 10, 2024

I updated a bunch of tests, but had to ignore a few due to some unfamiliarity and suspected disused code. This OWL API upgrade is important for maintaining OBO format parity with Protege and ROBOT.

@balhoff balhoff requested review from kltm and matentzn June 10, 2024 16:04
@balhoff
Copy link
Member Author

balhoff commented Jun 10, 2024

I don't think we have CI checks enabled here, but the remaining tests do all pass for me locally.

Copy link

@matentzn matentzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, I trust your own review much more. It looks ok - you skip a lot of tests, but I am assuming you dont do so without a good reason.

As I am anyways advocating for fading owltools out, I am fine with these changes which I eyeballed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming these different are accounted for!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My not too deep analysis made me think it is related to the new output of a label for oboInOwl:id, like noted here: owlcs/owlapi#1102 (comment)

@balhoff
Copy link
Member Author

balhoff commented Jun 10, 2024

@kltm if I merge this PR, would it immediately start getting built and used in the GO pipeline, or is the pipeline getting owltools from ODK now?

@kltm
Copy link
Member

kltm commented Jun 10, 2024

@balhoff Mixed bag, perhaps.
Ontology production uses the ODK ('obolibrary/odkfull:v1.4') in various branches. However, owltools also gets used for building the solr index and doing "predictions" in the pipeline; these seem to follow a local build aimed at master.

Copy link
Member

@kltm kltm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@balhoff eyeballing the changes, mostly adding ignores and updating endpoints, it seems to make sense. Everything else I would leave to your judgement.

@balhoff
Copy link
Member Author

balhoff commented Jun 11, 2024

@kltm I was thinking about the timing of things. I would merge this into master and then make an update to ODK. Once that is out the ontology build will start using it. But are you okay with the pipeline immediately using the new OWL API? I think it will be okay for the ontology, since it won't be saving out ontology files.

@kltm
Copy link
Member

kltm commented Jun 11, 2024

I'm not sure of the scope of any disruption, if any, as we're locked to ODK in the ontology build itself. Or, to ask another way, why would the solr loads or predictions creation change if this went in?

@balhoff
Copy link
Member Author

balhoff commented Jun 11, 2024

I'm not sure of the scope of any disruption, if any, as we're locked to ODK in the ontology build itself. Or, to ask another way, why would the solr loads or predictions creation change if this went in?

I don't think they would! Just wanted to check for any apprehension you may have. But it ought to be unchanged.

@kltm
Copy link
Member

kltm commented Jun 11, 2024

@balhoff Nah--my only ask would be that this is done either late today or early tomorrow, to give the current snapshot a chance to finish with a guaranteed version throughout the run.

@balhoff balhoff merged commit 50e2d32 into master Jun 12, 2024
@balhoff balhoff deleted the owlapi-upgrade branch June 12, 2024 17:31
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

Successfully merging this pull request may close these issues.

3 participants