piprate / json-gold

A JSON-LD processor for Go
Apache License 2.0
259 stars 30 forks source link

Refactor NormalisationAlgorithm to expose normalized quads #30

Closed joeltg closed 4 years ago

joeltg commented 4 years ago

This PR implements the proposal outlined in #26, with the goal of exposing a Quads(): []*Quad method on NormalisationAlgorithm that lets users access a sorted, normalized slice of quads. This would be very useful to projects using json-gold's Quad and Dataset structs as a general RDF library.

To recap, there are three changes:

  1. Add a Quads() []*Quad method to the NormalisationAlgorithm struct that just returns its internal .quads field.
  2. Change the way the dataset is sorted at the end of normalization so that the .quads slice is sorted in addition to the slice of string n-quads lines.
  3. Split most of the body of Main(dataset *RDFDataset, options *JsonLdOptions) (interface {}, error) into a separate method Normalize(dataset *RDFDatset) that performs normalization and sorting, but doesn't construct the concatenated result or return anything. Main(...) has the exact same interface and functionality, it just calls Normalize(dataset) and then constructs the return value based on the options, but now users who just want to use Quads() directly can just call Normalize().

I couldn't figure out how to run proper tests (like from the ld/testdata/normalization folder?), but testing informally with all the datasets I have lying produced correct results.

Thanks so much for maintaining this library; it's fantastic to have high-quality tooling for RDF start to become more common!

joeltg commented 4 years ago

A few more notes:

kazarena commented 4 years ago

@joeltg thank you for submitting this PR! The proposed changes make sense. I'm not overly concerned about naming, because of the reasons I outlined in #26.

Regarding the way to run normalisation tests, the official test suite for normalisation runs as a part of _ld/processortest.go. If you execute:

   make test

the whole test suite will run.

Thanks again! Will merge the PR now. I will add your name into the list of contributors with the next minor release.