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

RestrictionVisitor rewrite requires careful review #181

Open
cweedall opened this issue May 18, 2024 · 2 comments
Open

RestrictionVisitor rewrite requires careful review #181

cweedall opened this issue May 18, 2024 · 2 comments

Comments

@cweedall
Copy link
Contributor

I created a draft PR #180. There was a lot to cover and I included those discussion points in the PR.

The reason for this issue is because the code changes are likely not to be easily comparable. For example, RestrictionVisitor ended up be largely a fully rewrite (although, most of the code still remains, it was condensed and helper methods were used - therefore it looks very different overall).

After comparing three ontologies before the rewrite and using the rewrite .jar, everything seems to be generated as I expected. Mostly the same overall, including some bug fixes.

For some things (such as hasDegree for ProfessorInArtificialIntelligence referring to a subset [i.e. MS and PhD] of the Degree enum from example.owl), I made some assumptions and it renders the OpenAPI spec slightly differently. I may have made incorrect assumptions. Or, there may be a need/interest for both possibilities. I also would like help verifying if I made any errors.

I can mark the PR ready for review after any concerns are addressed. Thank you in advance!

@dgarijo
Copy link
Contributor

dgarijo commented May 20, 2024

Thanks, I will have a look as soon as I can!

@cweedall
Copy link
Contributor Author

cweedall commented Jun 25, 2024

Alright... it took longer than expected. A combination of life and trying to handle all the different ontology scenarios that I ran into. I utilized many of the changes from draft PR #180 into: draft PR #182.

This ended up being a major effort. I did my best to test it. But there may be things I missed.

The main thing is that complex properties are supported. So, if you see some values with a union or intersection, the property's items structure should be look similar to the following, for an object property:

propertyName:
  ...
  items:
    anyOf: #some values from
      anyOf/allOf: #union of / intersection of
        - $ref: "class reference"

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

2 participants