json-ld.net icon indicating copy to clipboard operation
json-ld.net copied to clipboard

API advertised in documentation/samples is now `internal`

Open sblom opened this issue 3 years ago • 8 comments

PR #77 marked RDFDatasetUtils as internal, which broke a couple of samples from our published documentation. There may be some other static utility methods that got hidden by this as well. I agree with the original intent of #76, but it looks like we need to put a little more thought into what use cases we intend to support.

/cc @goofballLogic

sblom avatar Oct 07 '20 02:10 sblom

Hmyea, I didn't realize we've already merged stuff in the 2.0.0 milestone. That's a bit unfortunate. We should probably branch out to support/1.x from before #77 was merged so we can create a new release from it that preserves backwards compatibility and then have master targeted at version 2.0.

asbjornu avatar Oct 07 '20 09:10 asbjornu

I'll try to do this branch today. Apologies for the oversight. @sblom I'll also check the docs for breakages today.

goofballLogic avatar Oct 09 '20 14:10 goofballLogic

this ok: https://github.com/linked-data-dotnet/json-ld.net/tree/support/1.x ? I'll add a PR to bring relevant changes over from master

goofballLogic avatar Oct 09 '20 15:10 goofballLogic

@sblom do you think we should support RDFDatasetUtils as a public API going forward? I can see the value in it but it seems like it ought to belong to a different repo - it's not really part of the core json-ld spec is it?

goofballLogic avatar Oct 09 '20 15:10 goofballLogic

this ok: https://github.com/linked-data-dotnet/json-ld.net/tree/support/1.x ?

Since it's branched out from be001896a72eef6a5e3251cb250a47de0550dc28, just before the merge of #77, it should be good. 👍

asbjornu avatar Oct 09 '20 21:10 asbjornu

@asbjornu, @sblom I believe that this is now fixed by the support/1.x branch. Except for the samples shown in:

  1. https://github.com/linked-data-dotnet/json-ld.net-Demo/blob/master/Sample6_ToRDF.cs
  2. https://github.com/linked-data-dotnet/json-ld.net-Demo/blob/master/Sample8_CustomRDFRender.cs
  3. https://github.com/linked-data-dotnet/json-ld.net-Demo/blob/master/Sample9_CustomRDFParser.cs

The design decision taken to expose RDF data as a custom RDFDataset type leaves us holding the bag for providing RDFDataUtils which can serialize this type to and from representations like NQuads.

The interface is also not uniform - in the sense that ToRDF will produce an object (which must be cast to RDFDataset), but FromRDF takes a string in NQuads format. Surely our API should use the same type so that data can be roundtripped?

I propose that:

  1. We remove any external notion of RDFDataset and RDFDatasetUtils and instead redefine ToRDF to return strings of N-Quads (as https://github.com/digitalbazaar/jsonld.js/#tordf-n-quads also does)
  2. We change the signature of IRDFParser to the following:
    public interface IRDFParser
    {
        string ParseToNQuads(JToken input);
    }

goofballLogic avatar Nov 15 '20 17:11 goofballLogic

I agree the current design is sub-optimal, @goofballLogic. But I'm not fond of returning string. Can we return an object that matches RDFDataset but is controlled by us?

asbjornu avatar Dec 04 '20 00:12 asbjornu

I don't quite understand - we already control RDFDataset - it's just a class which we authored. How is what you are suggesting any different?

FromRDF already accepts a string in NQuads format - doesn't it make sense to also return data this (standard) way rather than some custom class? We can always provide an extension library (like we are suggesting for System.Text.Json and Newtonsoft.Json) which enhances the text-based interface if people want the typed interaction.

goofballLogic avatar Dec 11 '20 15:12 goofballLogic