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

Corrected n3 method of path objects (fixes #2503) #2504

Merged
merged 3 commits into from Jul 31, 2023

Conversation

lukasmu
Copy link
Contributor

@lukasmu lukasmu commented Jul 30, 2023

Summary of changes

This is an attempt to fix #2503. Previously, the n3 method of path objects returned unexpected/invalid strings when the path objects were non-trivial (i.e. nested, consisting of property path groups). Examples can be found in the related issue and the tests.

This change fixes a bug. It might impact on users that previously relied on the invalid implementation of the n3 method. Not sure if/where this should be added to the documentation.

Checklist

  • Checked that there aren't other open pull requests for
    the same change.
  • Checked that all tests and type checking passes.
  • If the change has a potential impact on users of this project:
    • Added or updated tests that fail without the change.
    • Updated relevant documentation to avoid inaccuracies.
    • Considered adding additional documentation.
  • 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 Jul 31, 2023

Coverage Status

coverage: 90.926%. first build when pulling 70f1d90 on lukasmu:bugfix/path_n3_method into bdd574e on RDFLib:main.

Add return type annotation and some additional tests.
@aucampia aucampia added review wanted This indicates that the PR is ready for review ready to merge The PR will be merged soon if no further feedback is provided. fix Fixes an issue core Relates to core functionality of RDFLib, i.e. `rdflib.{graph,store,term}` concept: property path Relates to property paths as defined in the SPARQL spec. labels Jul 31, 2023
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.

Thanks for the pull request @lukasmu - it looks good to me. I added some additional tests that all passed, and a return type annotation.

@aucampia aucampia requested a review from a team July 31, 2023 23:22
@aucampia aucampia merged commit 9c73581 into RDFLib:main Jul 31, 2023
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concept: property path Relates to property paths as defined in the SPARQL spec. core Relates to core functionality of RDFLib, i.e. `rdflib.{graph,store,term}` fix Fixes an issue 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.

N3 method for path objects is bugged
3 participants