-
Notifications
You must be signed in to change notification settings - Fork 24
fix: Redesign scoped context in jsonld #750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. I've played with it a bit and it looks that nothing broke. Thanks a lot for looking into this. Let's resolve conflicts and get this merged! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
Closes #749
JSON-LD compaction goes through two phases, which we let
pyld
handle for us, namely:schema:isPartOf
get translated to full form IRI's likehttp://schema.org/isPartOf
. Only values/nodes that have a corresponding entry in the@context
or scoped@context
s inside the nodes/values themselves will get expanded, with other values getting droppedFor instance, the following JSON-LD document:
together with using the top-level context of the document
while being perfectly valid JSON-LD and a perfectly valid context, would delete the
topic
entry on compaction, since onlyinterest
andname
are specified in the context.topic
would be specified in the"@context": {"@vocab": "http://xmlns.com/foaf/0.1/"}
line when reading the document, but any context inside the document is not applied when compacting.So to not lose the
topic
field, we'd have to either manually add all fields of child nodes to the context supplied to the compaction function (leading to problems if you have the same name for two fields with a different Ontology, and making it quite hacky to know what has to be added to the context, which is what we were doing until now.Or we can use scoped (sub-)contexts, such as:
(See the
@context
added ininterest
, which is a scoped context that only applies tointerest
entries)If we supply this context to compaction, all fields are there successfully. Since we don't want two methods to calculate contexts, it makes sense to add this enhanced, nested context (instead of the current flat one) to the metadata files etc. as well. Since this leads to the whole context being at the top level of a document, we don't need the contexts inside the values (as in the original example) anymore, as that'd just be duplication.
This PR does exactly that, automatically expanding entries in the top-level context with their respective child-contexts, removing the contexts inside values.
Collection types already had code to propagate their contexts up, though the implementation was broken and the code never reached.
jsonld.ib
now has a newtype
parameter that allows the type (or fully namespaced string representation of the type to help with dependency hell) to be set for a property, which will automatically add it's context to the toplevel@context
on load.So for a collection, it's still (Example is for the
Dataset
class)with
Creator
being the class whose@context
will get added to theDataset
s@context
(ifDataset
is the top level element)And for single valued properties (one-to-one relations), like
project.creator
, it is nowwith
type=Creator
being new.Old contexts get automatically adjusted on loading (Since the code that caused all this now actually does its job) and if they're persisted will have the new JSON-LD. Child level property names don't have to be manually added to the top level context anymore.
As this is a rather drastic rewrite of how we handle JSON-LD, we should be really sure it works before merging.
Other changes:
Creator
has been moved to its own file, to prevent circular dependencies.1.1
, as otherwise scoped@context
are not available.Activity
as it's needed forCommitMixin
(This should not have to be done on every class inheriting fromCommitMixin
! we should fix this). This change is also in renku log to generate single identifier for dataset imports of the same resource #719, as it was needed in both places.