gbv / jskos-tools

Tools for working with the JSKOS data format.
https://gbv.github.io/jskos-tools/
MIT License
2 stars 0 forks source link

Add function to merge objects #14

Closed nichtich closed 5 years ago

nichtich commented 5 years ago

First check whether object type of two objects A and B match. Then append values from B to A unless they are single-valued (such as prefLabel.en) or complete arrays.

stefandesu commented 5 years ago

Two questions:

stefandesu commented 5 years ago

A more concrete example and a potential "special case" for merging:

Let's say there are three URIs that all belong to the same concept scheme: http://abc.de, http://def.de, http://ghi.de. Two objects (A and B) that are both this concept scheme are to be merged. Object A has the uri http://abc.de and identifier ["http://def.de"]. Object B has uri http://def.de and identifier["http://abc.de", "http://ghi.de"]. Whichuriandidentifier` should the resulting object have?

I'd say that:

What do you say?

stefandesu commented 5 years ago

Another question. You wrote:

First check whether object type of two objects A and B match.

What if one of the two objects doesn't contain a type property? Would you throw an error if both objects contain type, but they are different (e.g. one is a concept, one a concept scheme)? Should this only apply to known JSKOS types or in general (probably only to known types)?

nichtich commented 5 years ago

The merge function should have options to differentiate. In general object properties and arrays (named as "set" in JSKOS for good reason) should be merged, see JSKOS sets how to handle sets of objects.

In the case of conflicting values (e.g. uri, prefLabel.en, startDate...), the first object should win but an error could be throws for selected fields if they don't match. Field uri is a special case: on request move the differing URI of object B into identifiers array as suggested above.

If guessObjectType cannot tell what an object is, it should be save to assume that it can be merged.

stefandesu commented 5 years ago

I implemented a first version of the method (but without creating a new release yet). See the implementation and documentation of the method, and especially the test cases whether this works as you would expect. Feel free to add more test cases. If anything is missing or wrong, adjust the test cases and commit them to a separate branch (so that Travis won't fail on master), and I will change the implementation to fix it. If everything is alright, please close the issue. 👍

nichtich commented 5 years ago

How about splitting merge into independent functions matchObjectTypes, mergeProperties, mergeUris and don't call matchObjectTypes from merge, so we don't need option throwOnTypeMismatch. It makes no sense to merge objects of different type anyway but the caller should knwo what he/she is merging, no?

nichtich commented 5 years ago

I'd also rename option uriMerge to mergeUris and option throwOnMismatch to detectMismatch.

stefandesu commented 5 years ago
nichtich commented 5 years ago

Strictly reading the JSKOS spec, arrays should only be merged if the first array ends with null but we can later add such an option if needed.

stefandesu commented 5 years ago

I merged your branch and adjusted everything as discussed. Is there anything I've missed?

Strictly reading the JSKOS spec, arrays should only be merged if the first array ends with null but we can later add such an option if needed.

Haven't thought about that, but that's true. In reality though, different sources have different information about the same object and arrays need to be merged anyway. I don't think I've seen a single case so far where an array has elements ending with null. Either the elements were there already, or there's only null and the elements are either unknown or loaded from an API.

stefandesu commented 5 years ago

Version 0.2.3 with merge is now released.