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 for issue 893 #1501

Merged
merged 2 commits into from Dec 13, 2021
Merged

Fix for issue 893 #1501

merged 2 commits into from Dec 13, 2021

Conversation

ghost
Copy link

@ghost ghost commented Dec 12, 2021

As iddan reports in issue #893Dataset is unpickeld as ConjunctiveGraph because it extends ConjunctiveGraph and inherits the __reduce__() method. It should be unpickled as Dataset instead.”

A fix was provided but unfortunately overlooked. Rectifying this oversight.

#893, courtesy of Iddan Aaronsohn
@ghost
Copy link
Author

ghost commented Dec 12, 2021

Um. Needs some investigation. Looks like the fix works but the test needs some refinement.

@ghost
Copy link
Author

ghost commented Dec 12, 2021

Probably don't need to keep the test as part of the standard suite, can be removed later, it's not as if it's going to be subject to regression.

@nicholascar nicholascar self-requested a review December 13, 2021 02:22
@nicholascar nicholascar merged commit 5c65c38 into RDFLib:master Dec 13, 2021
@aucampia
Copy link
Member

@ghost
Copy link
Author

ghost commented Dec 13, 2021

Seems this is failing sporadically:

* https://github.com/RDFLib/rdflib/runs/4501225806?check_suite_focus=true

* https://github.com/RDFLib/rdflib/runs/4505918939?check_suite_focus=true

Hmm. Investigating.

@nicholascar
Copy link
Member

Yes, I thought it was fine to merge as tests passed but then I too notice the intermittent failing.

If we can sort this out shortly, I'll release 6.0.3 with what we have. There seems to be no end of new, very good, PRs but I want a 6.0.3 out before the work on Graph IDs that @gjhiggins is leading as that's a large item and it would be safe to make a stable release just before it.

@aucampia
Copy link
Member

I think this does fix it: #1504 - and makes sense as a cause for the sporadic failure.

@ghost ghost mentioned this pull request Dec 19, 2021
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