-
Notifications
You must be signed in to change notification settings - Fork 560
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
Generate JSON-LD context from a DefinedNamespace #1706
Conversation
I'm curious about the use case here (especially why just using |
See the profile resources for GeoSPARQL 1.1: https://github.com/opengeospatial/ogc-geosparql/tree/master/1.1. It includes context/geo-context.json which is a JSON-LD context file for all the GeoSAPRQL elements. I want this function to enable I find that for new ontologies, I'm making |
I can see that this means of providing a hint of what properties are available may be useful. It is somewhat limited (as it provides no hint about what values to expect for instance), and I'd suspect users to either need the full ontology, or some (ideally generated) JSON schema du jour, depending on further implementation needs. I'm not sure that it fits as a method on a core RDFLib class though? I wouldn't expect users of these instances to need a method for it? Also, it does in principle collide with a vocabulary property of the same name (quite unlikely of course, but conceivable). I believe this function may fit better in some utility or tool. Perhaps in rdflib/tools alongside the script you referenced. On that topic, you may want to check out this old vocabulary to JSON-LD context generator made for schema.org quite some time ago. |
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.
Changes look good to me.
To me this seems like a useful thing to be able to do from a namespace, I think there is of course more that can be done from an ontology but I still think this does not really go way outside of purpose of RDFLib. It is maybe not that difficult to implement on top of RDFLib but it is also not a significant liability to maintain in RDFLib either. Some utilities to take any RDF graph and/or OWL graph and generate a context for a prefix may also be useful, but I think even so this makes sense. |
If you intend to keep it as a method rather than a utility function, I'd probably recommend making it "protected" with a leading underscore to lesser the collision risk, and possibly just returning an unserialized dict for the user to serialize in any preferred form. A bit like |
Actually, maybe better as a free function inside rdflib.namespace yes. |
Co-authored-by: Iwan Aucamp <[email protected]>
Co-authored-by: Iwan Aucamp <[email protected]>
Co-authored-by: Iwan Aucamp <[email protected]>
rdflib/namespace/__init__.py
Outdated
@@ -239,6 +239,16 @@ def __dir__(cls) -> Iterable[str]: | |||
values = {cls[str(x)] for x in cls.__annotations__} | |||
return values | |||
|
|||
def jsonld_context(self, pfx: str) -> str: |
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.
This still would collide with any jsonld_context
term in a namespace. It should thus at least be renamed or moved to a utility function. It also makes sense to let it return a dict, to allow the user of the function to decide serialization form (indent, sorting, etc.).
I've change the function name to |
Will try do a review this weekend and fix tests if they still fail. |
Method name changed and it now returns a dict instead of a JSON string.
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 good to me.
Closes #1705
Proposed Changes
jsonld_context()
function to theDefinedNamespace
classjsonld_context()
functionNotes for reviewers: I don't really get what the purpose of the
DefinedNamespaceMeta
class is v. theDefinedNamespace
class. There is some funky Python going on there that is beyond me. Is it best to have added this function to DefinedNamespaceMeta, rather than
DefinedNamespace? It still appears for use with
DefinedNamespaceand that's where all the 'DefinedNamespace
functions seem to be...