Feature/issue-40#41
Feature/issue-40#41asbjornu merged 13 commits intolinked-data-dotnet:masterfrom dnllowe:feature/issue-40
Conversation
|
|
||
| private bool produceGeneralizedRdf = false; | ||
|
|
||
| private bool sortGraphsFromRdf = true; |
There was a problem hiding this comment.
Since I've only been able to control the order with the FromRDF method (which luckily is all that's needed to get the desired behavior), I made the option pretty narrow in scope.
| { | ||
| [Theory, ClassData(typeof(ConformanceCases))] | ||
| public void ConformanceTestPasses(string id, string testname, ConformanceCase conformanceCase) | ||
| public void ConformanceTestPasses(string id, ConformanceCase conformanceCase) |
There was a problem hiding this comment.
The testname variable was never used, so removing it had no impact
|
|
||
| public IEnumerator<object[]> GetEnumerator() | ||
| { | ||
| var jsonFetcher = new JsonFetcher(); |
There was a problem hiding this comment.
I created the JsonFetcher class so I could reuse some of this logic in my own test class
Codecov Report
@@ Coverage Diff @@
## master #41 +/- ##
==========================================
+ Coverage 68.38% 68.59% +0.20%
==========================================
Files 22 22
Lines 4043 4053 +10
Branches 1026 1028 +2
==========================================
+ Hits 2765 2780 +15
+ Misses 1133 1130 -3
+ Partials 145 143 -2
Continue to review full report at Codecov.
|
| } | ||
| } | ||
|
|
||
| private JToken GetJson(JToken j) |
There was a problem hiding this comment.
Moved to the JsonFetcher class
| @@ -0,0 +1,89 @@ | |||
| [ | |||
There was a problem hiding this comment.
After getting FromRDF to work, I tried the same approach with Compact, but it seemed too complicated to get it to work. Plus, if Compact or the other methods are given input, they seem to maintain the order. I left the remnants of the test here though in case it seemed worthwhile to try again later.
Or we can just delete it.
| @@ -0,0 +1,58 @@ | |||
| { | |||
There was a problem hiding this comment.
This was the easiest way for me to create quads to provide the FromRDF method.
| @@ -0,0 +1,143 @@ | |||
| using JsonLD.Core; | |||
There was a problem hiding this comment.
I wanted to keep the sorting tests self-contained and not mix them in with the other tests, assuming that if the other tests are serializing correctly, and this is sorting correctly, then all is well.
|
It's important to maintain the same API as that exposed by jsonld.js can you check what they are doing on this issue? |
|
@dnllowe, do you think we could move this PR forward by answering @goofballLogic's question? Looks like @xivk's review would be appreciated as well. |
Sorry--the company had ended up forking the repository for the change. But let me take a look this weekend and get back to you! |
|
The |
|
sounds like the jsonld guys are on the fence about this functionality. Is there a way we could distinguish between "conformance" tests and "extra functionality" tests in our suite - so that there's a clear distinction between the official suite's tests and this extra function? |
That sounds reasonable. Could go with either |
|
|
Hey, no worries at all-and no rush! You've already been a great help, and I appreciate it :) |
Moves to ExtendedFunctionalityTests Refactors to create room for more extended functionality tests
|
Is this a breaking change so we should consider bumping the major version after merging this? |
I think this is adding a predictable behaviour (sorting) where previously the behaviour was, in theory, unpredictable. Is it possible that somebody could depend on things not being sorted??? |
|
I agree that sounds highly unlikely, @goofballLogic. Let's merge and bump minor. |
4 participants
My team and I have been using this library and it's made life much easier when working with quads and the rdf format. Thanks!
However, we have a need to return jsonld where order matters. But, the
JsonLdApi.FromRDFmethod sorts properties by default. We need to be able to control when to sort the top-level graphs, the child nodes, or both.The new
SortingTestsprovide examples for what we're after, but as a summary, given the json below, we'd want to control whether nodes 1, 2, and 3 get sorted, and whether their children (objects 1, 2, and 3) are sorted. As of now, theJsonLdApi.FromRDFwill always sort the graph nodes and children, losing the orignal order.[ { "@id": "http://example.org/node/3", "@graph": [ { "@id": "http://example.org/object/3", "http://example.org/value": [ { "@id": "n3-o3-value" } ] }, { "@id": "http://example.org/object/1", "http://example.org/value": [ { "@id": "n3-o1-value" } ] }, { "@id": "http://example.org/object/2", "http://example.org/value": [ { "@id": "n3-o2-value" } ] } ] }, { "@id": "http://example.org/node/1", "@graph": [ { "@id": "http://example.org/object/3", "http://example.org/value": [ { "@id": "n1-o3-value" } ] }, { "@id": "http://example.org/object/1", "http://example.org/value": [ { "@id": "n1-o1-value" } ] }, { "@id": "http://example.org/object/2", "http://example.org/value": [ { "@id": "n1-o2-value" } ] } ] }, { "@id": "http://example.org/node/2", "@graph": [ { "@id": "http://example.org/object/3", "http://example.org/value": [ { "@id": "n2-o3-value" } ] }, { "@id": "http://example.org/object/1", "http://example.org/value": [ { "@id": "n2-o1-value" } ] }, { "@id": "http://example.org/object/2", "http://example.org/value": [ { "@id": "n2-o2-value" } ] } ] } ]I've gotten this to work only with
JsonLdApi.FromRDF, but that's the only need we have, so I've left the rest of the code alone as much as possible. Testing the same changes withCompactrevealed just how complicated it might be to try and control the order in all cases--but if the original input for the other methods (Compact,Flatten,Expand, etc) is in the order that's desired, then it seems to remain unchanged. It was onlyFromRDFthe changed the order of the input.Fixes #40.