Skip to content

fix: bugs with rdflib.extras.infixowl - #2390

Merged
aucampia merged 25 commits into
RDFLib:mainfrom
aucampia:aucampia/20230519T1021-infix-fixes
May 19, 2023
Merged

aucampia merged 25 commits into
RDFLib:mainfrom
aucampia:aucampia/20230519T1021-infix-fixes

Conversation

@aucampia

@aucampia aucampia commented May 19, 2023 •

Copy link
Copy Markdown
Member

Summary of changes

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:

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 granting push permissions to the PR branch,
    so maintainers can fix minor issues and keep your PR up to date.

Graham Higgins and others added 24 commits March 22, 2023 13:50
tidy `# docstring` syntax and replace antiquated namespace_manager bindings with moderm graph ns bindings.
`subject_node_closure` and `blank_node_closure` don't have tests and are
not essential to the rest of the PR. Removing them so the rest can be
merged.
Removing this would be a breaking change, it should be marked as
deprecated instead and if it is being replaced by another function then
it should call the other function.
This will invalidate the xfail if the exception being raised changes,
which makes it a bit safer to use xfail, as it is less likely to mask
other newly introduced bugs.
@aucampia

aucampia commented May 19, 2023 •

Copy link
Copy Markdown
Member Author

@gjhiggins please see if you are okay with the changes I made. All changes can be seen here:

Of particular import may be:

  • 5130e36

    Add back Individual.serialize

    Removing this would be a breaking change, it should be marked as
    deprecated instead and if it is being replaced by another function then
    it should call the other function.

Comment thread rdflib/extras/infixowl.py Outdated
@aucampia
aucampia marked this pull request as ready for review May 19, 2023 08:53
@aucampia
aucampia requested review from a user May 19, 2023 08:53
@aucampia aucampia added the review wanted This indicates that the PR is ready for review label May 19, 2023
@coveralls

coveralls commented May 19, 2023 •

Copy link
Copy Markdown

Coverage Status

Coverage: 90.85% (-0.004%) from 90.854% when pulling e65e721 on aucampia:aucampia/20230519T1021-infix-fixes into e103078 on RDFLib:main.

@ghost ghost left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍 (and thank you)

As far as I know this is not a documented dunder, and according to
PEP-8
\[[ref](https://peps.python.org/pep-0008/#descriptive-naming-styles)\]:

> Never invent such names; only use them as documented.
@aucampia
aucampia force-pushed the aucampia/20230519T1021-infix-fixes branch from f99f085 to e65e721 Compare May 19, 2023 10:05
@aucampia aucampia added the ready to merge The PR will be merged soon if no further feedback is provided. label May 19, 2023
@aucampia
aucampia merged commit cd0b442 into RDFLib:main May 19, 2023
@aucampia aucampia mentioned this pull request May 19, 2023
8 tasks done
@aucampia
aucampia deleted the aucampia/20230519T1021-infix-fixes branch June 8, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready to merge The PR will be merged soon if no further feedback is provided. review wanted This indicates that the PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants