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

Fix 939 return cg from parse #2563

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

mielvds
Copy link
Contributor

@mielvds mielvds commented Aug 31, 2023

Summary of changes

Fixes #939, which was really easy to do since most of the logic moved out of the CG parse method. Not sure whether this needed additional discussion or not; it does change the public API

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 adds new features or changes the RDFLib public API:
    • Created an issue to discuss the change and get in-principle agreement.
    • Considered adding an example in ./examples.
  • 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 Aug 31, 2023

Coverage Status

coverage: 91.018% (+0.01%) from 91.008%
when pulling 0ed7909 on mielvds:fix-939-return-cg-from-parse
into a8f37cc on RDFLib:main.

@aucampia
Copy link
Member

pre-commit.ci autofix

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.

@mielvds thanks for this, this is a breaking change, so I will hold off a little with merging it, as I want to release 7.0.1 or 7.1 in the next weeks, but I will merge it soon afterwards.

@aucampia aucampia added the backwards incompatible will affect backwards compatibility, this includes things like breaking our interface label Aug 31, 2023
@ashleysommer
Copy link
Contributor

Note, with impending removal of ConjuntiveGraph, I don't think this is necessary.

Copy link
Member

@nicholascar nicholascar left a comment

Choose a reason for hiding this comment

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

Happy to approve this but for the RDFLib 8.0.0 release (Sept 2024??) in since the major version number change will be appropriate for the API change.

We will need to remove the ConjunctiveGraph handling though as CG will be removed in 8.0.0, so we will end up with just Dataset for quad graphs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible will affect backwards compatibility, this includes things like breaking our interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rdflib 4.2.2: ConjunctiveGraph.parse() return a Graph object
5 participants