Ld_list issues

[notactuallyfinn]

While testing ld_list in #418 we found some potential issues:

• Sets and graphs are not dictionaries with "@set" or "@graph" as keys for their list of items in their expanded JSON-LD form. But the constructor of and is_ld_list from ld_list assume it otherwise (is_container of ld_list has the same flaws) and the conversion from a ld_list with container = "@set" to expanded json gives the wrong form:
  • {"https://schema.org/name": {"@set": ["a", "b"]}}'s expanded version is [{"https://schema.org/name": [{"@value": "a"}, {"@value": "b}]}].
  • is_ld_list([{"@set": ["a", "b"]}]) and is_ld_list([{"@set": [{"@value": "a"}, {"@value": "b}]}]) return True but is_ld_list([{"@value": "a"}, {"@value": "b}]) returns False
  • ld_list([{"@set": [{"@value": "a"}, {"@value": "b}]}]) returns the correct object but ld_list([{"@set": ["a", "b"]}]) creates a non expanded ld_list and ld_list([{"@value": "a"}, {"@value": "b}]) raises a ValueError
  • ld_list.from_list(["a", "b"], container="@set", key="https://schema.org/name", context={"schema": "https://schema.org/"}).ld_value is [{"@set": [{"@value": "a"}, {"@value": "b}]}] instead of [{"@value": "a"}, {"@value": "b}]}]
• Should a list containing a single value (a dictionary) be considered an expanded set/ graph or just as a expanded JSON-LD dict if it isn't a value or id? This is relevant for e.g. setting or appending such an object (for example [{"schema:name": "foo"}]) but also for ld_dict.
• JSON-LD supports nested containers. But the type of those makes a difference which is not considered in __setitem__ and append of ld_list. For example:
  • A set inside a set gets unpacked but a set inside a list gets turned into a list when expanded.
  • [{"https://schema.org/name": [{"@set": [{"@value": "a"}, {"@set": ["b", "c"]}, {"@list": ["d", "e"]}]}]}]'s expanded from is [{"https://schema.org/name": [{"@value": "a"}, {"@value": "b"}, {"@value": "c"}, {"@list": [{"@value": "d"}, {"@value": "e"}]}]}]
  • [{"https://schema.org/name": [{"@list": [{"@value": "a"}, {"@set": ["b", "c"]}, {"@list": ["d", "e"]}]}]}]'s expanded from is [{"https://schema.org/name": [{"@list": [{"@value": "a"}, {"@list": [{"@value": "b"}, {"@value": "c"}]}, {"@list": [{ "@value": "d"}, {"@value": "e"}]}]}]}].
• Should parent, key, index and context be also compared when comparing two ld_lists with == through __eq__ or with != through __ne__? At the moment only the itemlist and the container type are is compared. And should the order be ignored for ld_lists with container type "@set"?
• __init__ documents a parameter that does not exist.

[notactuallyfinn]

We discussed today that ld_list should no longer be explicitly representing "@set" and "@graph". Instead its content should be treated as if it was an "@list" (ordered list).

Explicitly representing "@set" is not necessary because the only difference to "@list" is that "@set" is explicitly unorded and treating it as if it was ordered changes nothing.
Explicitly representing "@graph" does not seem necessary because our default graph only contains one item and is represented by a SoftwareMetadata object and we do not see a need for any other graph in our context.

This also solves the first and third problem mentioned in this issue.

[notactuallyfinn]

As for the second problem, I would say that we should treat it like a list.
We probably always return a list anyway (decision not finalized yet) when we get an object from an ld_dict. Therefore, it makes no difference how we insert the object in question into an ld_dict.
If you insert/append the object to a list, I think it's more likely to be a list (e.g. if the iteration yielded only one element) than an expanded dict (because for the user it's easier to create a non-expanded dict).

[led02]

We discussed today that ld_list should no longer be explicitly representing "@set" and "@graph". Instead its content should be treated as if it was an "@list" (ordered list).

Explicitly representing "@set" is not necessary because the only difference to "@list" is that "@set" is explicitly unorded and treating it as if it was ordered changes nothing. Explicitly representing "@graph" does not seem necessary because our default graph only contains one item and is represented by a SoftwareMetadata object and we do not see a need for any other graph in our context.

This also solves the first and third problem mentioned in this issue.

Voice from the off...

First, I want to emphasize that the ld_container class hierarchy is was initially meant as a helper to better cope with expanded JSON-LD. The idea behind the data model is, that we have valid expanded JSON-LD and these classes are a small utility to help handling them. The guts of HERMES processing should not rely on these classes to follow the JSON-LD specs.

However, small helper classes grow and get almighty and now I would really see a high potential of refactoring them out into some general purpose untility library. Until then, we should look at them with more pragmatic eyes:

• What is the purpose of these classes? I don't think (for now), that they should achieve full JSON-LD compliance. This is especially important for expansion rule of the right hand side of a node. Regading this discussion: Stick with what is currently working. And instead of aiming for the highest possible test coverage, start using the classes and document misbehaviour and or unintuitve use in form of a test case... and then take a decision whehter to fix this behaviour.
• Who is the target audience? Well, it is in case the user - but the use must be aware of what he is dealing with (i.e., expanded JSON-LD data behind a facade that makes it more usable). This means, if in doubt the primary data that needs to be handled is the raw JSON-LD dict. Hence, we also need to guarentee that the self.data attribute is always a refernce to the actual data set...

Having clearified that, her are my 2ct regarding the actual contents of this ticket:

• A @set needs to ensure that there are no duplicates, hence the items need to be comparable (i.e., having and @id). But the @graph can be treated as an unordered list. I aggre, there is a confusion between the class name and the JSON-LD terms here. Yet, I'd propose to rename the Python class instead of introducing additional fuzz by adding other classes that add no semantics. (I would also stay away from adding semantics here before these classes are its own project... KISS)
• Not wrapping a sequence into a value-container is totally fine but has complicated semantics on how to convert it... so I would always wrap it...
• Please keep in mind that these classes are wrappers and do not convert things... they should not even copy stuff!
  I.e., given the expanded JSON-LD [{"foo": [{"@list": [{"@set": [{"a": "b"}]}]}] as ld_list a, then a["foo"] is a ld_list with a.["foo"].data being [{"@list": [{"@set": [{"a": "b"}]}]}]. Accessing a["foo"][0] again returns a ld_list which is created just when the access operation is done.

PS: I'll leave my comments in the MR, too ;)

[notactuallyfinn]

Having clearified that, her are my 2ct regarding the actual contents of this ticket:

• A @set needs to ensure that there are no duplicates, hence the items need to be comparable (i.e., having and @id). But the @graph can be treated as an unordered list. I aggre, there is a confusion between the class name and the JSON-LD terms here. Yet, I'd propose to rename the Python class instead of introducing additional fuzz by adding other classes that add no semantics. (I would also stay away from adding semantics here before these classes are its own project... KISS)
• Not wrapping a sequence into a value-container is totally fine but has complicated semantics on how to convert it... so I would always wrap it...
• Please keep in mind that these classes are wrappers and do not convert things... they should not even copy stuff!
  I.e., given the expanded JSON-LD [{"foo": [{"@list": [{"@set": [{"a": "b"}]}]}] as ld_list a, then a["foo"] is a ld_list with a.["foo"].data being [{"@list": [{"@set": [{"a": "b"}]}]}]. Accessing a["foo"][0] again returns a ld_list which is created just when the access operation is done.

• {"@context": "http://schema.org/", "@type": "Person", "name": [{"@set": [{"@id": "A"}, {"@id": "A"}]}]} is expanded to [{"@type": ["http://schema.org/Person"], "http://schema.org/name": [{"@id": "A"}, {"@id": "A"}]}] (proof). This means @set does not require it contents to be unique, i.e. it can contain duplicates. The reason for removing @set and @graph and always replacing it with @list wasn't the name of the class ld_list. (And it wasn't planned to add other classes like ld_set.)
• If you mean removing the key @set and the dict and only storing the actual value: I think that is a must because otherwise it is not valid JSON-LD. Anyhow it is neccessary to convert from that state to our python versions as ld_dict.from_dict uses the JSON-LD-Processor to expand the given dict that may contain any valid JSON-LD like a non-expanded set.
• Your given "expanded" JSON-LD is not valid it should be [{"http://schema.org/foo": [{"@list": [{"@list": {"http://schema.org/a": [{"@value": "b"}]}]}]}]}] instead (proof). But anyhow you are right.