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

Infixowl fix #2307

Closed
wants to merge 23 commits into from
Closed

Infixowl fix #2307

wants to merge 23 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 23, 2023

Summary of changes

  • added example of ontology creation using infixowl syntax
  • updated infixowl tests
  • added _rdfList class variable
  • fixed cardinality issue
  • fixed Callable issue
  • added documentation of operator mapping
  • robustified return of string
  • modernized OWLRDFListProxy class def
  • added documentation to IIndividual
  • renamed Individual.serialize to Individual.snc (subject node closure)
  • added Individual.bnc (blank node closure)
  • changed to use @ throughout as infix operator
  • tidied # docstring syntax
  • replaced antiquated namespace_manager bindings with moderm graph ns bindings.

Checklist

  • Checked that there aren't other open pull requests for the same change.
  • Added tests for any changes that have a runtime impact.
  • Checked that all tests and type checking passes.
  • For changes that have a potential impact on users of this project:
    • Updated relevant documentation to avoid inaccuracies.
    • Considered adding additional documentation.
    • Considered adding an example in ./examples for new features.
    • Considered updating our changelog (CHANGELOG.md).
  • Considered granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

@coveralls
Copy link

coveralls commented Mar 23, 2023

Coverage Status

Coverage: 90.824% (-0.03%) from 90.85% when pulling 8ca2dc5 on gjhiggins:infixowl-fix into ddcc4eb on RDFLib:main.

rdflib/extras/infixowl.py Outdated Show resolved Hide resolved
rdflib/extras/infixowl.py Outdated Show resolved Hide resolved
Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

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

@gjhiggins looks good with some minor comments. I'm happy to address them in your branch.

@aucampia
Copy link
Member

@gjhiggins looks good with some minor comments. I'm happy to address them in your branch.

@gjhiggins just a follow up on this, I will update your branch accordingly this weekend if there are no objections.

@aucampia aucampia closed this Apr 12, 2023
@aucampia aucampia reopened this Apr 12, 2023
@aucampia
Copy link
Member

Sorry for the accidental close

@aucampia aucampia self-assigned this Apr 16, 2023
@ghost
Copy link
Author

ghost commented Apr 16, 2023

@gjhiggins looks good with some minor comments. I'm happy to address them in your branch.

@gjhiggins just a follow up on this, I will update your branch accordingly this weekend if there are no objections.

Sure, no problem, thank you.

@ghost
Copy link
Author

ghost commented May 9, 2023

Mm. Wonder why 3.11 is failing on

graph.parse(location="http://www.w3.org/ns/adms.ttl")

@aucampia
Copy link
Member

Mm. Wonder why 3.11 is failing on

graph.parse(location="http://www.w3.org/ns/adms.ttl")

This is unrelated to this PR.

@aucampia
Copy link
Member

Mm. Wonder why 3.11 is failing on

graph.parse(location="http://www.w3.org/ns/adms.ttl")

This is unrelated to this PR.

I figured it out:

Looking at a way to fix it now.

@aucampia aucampia added the awaiting feedback More feedback is needed from the author of the PR or Issue. label May 17, 2023
aucampia added a commit that referenced this pull request May 19, 2023
Fix the following issues in `rdflib.extras.infixowl`:
- getting and setting of max cardinality only considered identifiers and not other RDF terms.
- The return value of `manchesterSyntax` was wrong for some cases.
- The way that `BooleanClass` was generating its string representation (i.e. `BooleanClass.__repr__`) was wrong for some cases.

Other changes:
- Added an example for using infixowl to create an ontology.
- Updated infixowl tests.
- Updated infixowl documentation.

This code is based on code from:
- <#2307>

Changes are primarily authored by <https://github.com/gjhiggins>.

---------

Co-authored-by: Graham Higgins <gjh@bel-epa.com>
@aucampia
Copy link
Member

@gjhiggins this now only contains changes for the node closure function additions as the rest was merged in #2390

@aucampia aucampia removed the awaiting feedback More feedback is needed from the author of the PR or Issue. label May 19, 2023
@ghost
Copy link
Author

ghost commented May 19, 2023

@gjhiggins this now only contains changes for the node closure function additions as the rest was merged in #2390

Noted, thanks. Bizarrely, GH still reporting that the branch in my clone repos is 22 commits ahead, sigh.

@aucampia aucampia removed their assignment May 19, 2023
@aucampia
Copy link
Member

Noted, thanks. Bizarrely, GH still reporting that the branch in my clone repos is 22 commits ahead, sigh.

If you do a rebase, that should possibly improve. A squash using git rebase --interactive upstream/main may also make sense.

@ghost
Copy link
Author

ghost commented May 19, 2023

Noted, thanks. Bizarrely, GH still reporting that the branch in my clone repos is 22 commits ahead, sigh.

If you do a rebase, that should possibly improve. A squash using git rebase --interactive upstream/main may also make sense.

Thanks for the advice but sadly, I'm lost in my ignorance of git at this point, will have to find time to spend on understanding how rebase and squash work.

@ghost
Copy link
Author

ghost commented May 19, 2023

Thanks for the advice

Something profoundly amiss, can't recover a sane branch, I'm closing this PR, will delete the problematic branch and will raise another PR once I've created some tests.

@ghost ghost closed this May 19, 2023
@ghost ghost deleted the infixowl-fix branch May 19, 2023 16:52
This pull request was closed.
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.

None yet

2 participants