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

JSON-LD Integration #1354

Merged
merged 13 commits into from
Jul 12, 2021
Merged

JSON-LD Integration #1354

merged 13 commits into from
Jul 12, 2021

Conversation

ashleysommer
Copy link
Contributor

This is a big one.
There are 2883 files changed. But most of those are new files forming the giant JSON-LD test-suite (and a 2nd copy of it, used by the JSON-LD v1.1 test-suite).

This integrates the json-ld library into RDFLib as a core parser/serializer. Specifically, it uses the feature/v1.1 branch, which includes experimental support for basic JSON-LD v1.1 compliance.

It automatically registers the json-ld parser and serialize against the "json-ld" file format, and the application/ld+json mimetype.

Lots of tests are not working, for a couple of reasons:

  • There are still RDFLib idiosyncrasies that prevent json-ld from parsing or serializing exactly as per the spec
  • The v1.1 features are not yet complete (lots of v1.1 tests are broken)

All of the failing tests have been investigated, and commented on (see comments in the known_bugs section of the test suite).
For now, all known bugs which cause test failures are skipped, in order to keep the green light on in github.

I've removed from docs any mention of installing JSON-LD as a library, and added mention of JSON-LD as a built-in parser and serializer.

I've added mention of this change in the 5to6upgrade docs.

@ashleysommer
Copy link
Contributor Author

Lots of failing tests in Drone, not sure whats going on, I'll check it out

@ashleysommer
Copy link
Contributor Author

ashleysommer commented Jul 8, 2021

I just remembered we should check licence compatibility between rdflib-jsonld and rdflib, to ensure we're legally allowed to merge the code into this codebase.

Good news. Both projects use the BSD-3-Clause license, so they're completely compatible. I'm pretty sure we don't any licence reason we can't merge this.

@nicholascar
Copy link
Member

Nice work @ashleysommer! I guess I’ll wait for you to have another go at fixing the failing tests before reviewing properly?

@nicholascar
Copy link
Member

Can you just confirm that all these JSON-LD tests are excluded from pip installs of the module? I.e even if we add a million files, it only affects cloning not pip use since the test files are excluded from the PyPI package?

@ashleysommer
Copy link
Contributor Author

I'll double check about the files. They're all in the test directory, and I think that whole directory is excluded by default.

@ashleysommer
Copy link
Contributor Author

I don't know whats up with the tests. There are some failing that are completely unrelated to the changes I made. I'll merge master back in and re-test it.

@ashleysommer
Copy link
Contributor Author

Ok, I found the problem with the tests. Turns out the jsonld tests were changing the working directory with chdir() and never changing it back, which breaks all subsequent tests.

Regarding the files: They are bundled in the sdist package (as is the case with all of the rdflib test suite files), and not included in the bdist or the bdist_wheel packages. So I think that is correct.

@ashleysommer
Copy link
Contributor Author

Tests are still failing. Turns out the json-ld library doesn't play nicely with the rdflib 6.0 isomorphic graph comparison, and breaks some tests elsewhere in the test suite.

@nicholascar
Copy link
Member

We might be hitting some of the limits of the way the JSON-LD testing occurs: by char-by-char comparisons of output v. input when RDF could be in any order. I had to update the BN naming MonkeyPatch for 6.0.0ao already (https://github.com/RDFLib/rdflib-jsonld/blob/feature/json-ld-1.1/test/runner.py#L14) and did get all existing tests to pass but likely other issues are related to this sort of thing?

@ashleysommer
Copy link
Contributor Author

I've fixed most of the tests. The remaining ones are caused by the preserving-bnode monkeypatch in the json-ld test suite. It is not un-patched after tests are run.
There are some N3-parser tests that specifically require the BNode IDs to not be preserved.

@ashleysommer
Copy link
Contributor Author

Yeah! Tests passed. @nicholascar @edmondchuc Review and Merge requested.

@edmondchuc edmondchuc modified the milestone: rdflib 6.0.0 release Jul 9, 2021
@nicholascar
Copy link
Member

@edmondchuc if you're happy, approve and directly merge!

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.

3 participants