-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update SPARQL in functions.py #47
base: master
Are you sure you want to change the base?
Conversation
Suppress references to unused namespaces and deprecated predicates
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 looks like a good start. We will need to build the new cache and test this code pretty thoroughly before we can merge it.
Yes I wonder if |
rdf:predicate rdf:type ; | ||
rdf:object prov:Location . | ||
} . | ||
# UNION |
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.
My best guess as to why this was here in the first place is because "overlaps" objects are treated as geo:Features but we want to exclude them from being returned as locations. I'm not sure though, @ashleysommer ?
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.
Are these computed ahead and cached? If so, I would prefer to see an explicit feature-type for overlaps. Maybe loci:Overlap
? Then these can be explicitly excluded, rather than borrowing an unrelated feature-type.
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.
Yes I agree it shouldn't be a feature but I can't remember all the reasoning around it from our older discussions. I believe we can actually get rid of overlaps objects altogether because all that information should be elsewhere but @ashleysommer has more history and probably more insight on this. We need to break up scope here as well. It would be good to merge whatever is the minimum set of changes to support the new LDAPIs and deal with larger refactors, like overlaps, later.
functions.py
Outdated
PREFIX geox: <http://linked.data.gov.au/def/geox#> | ||
PREFIX qb4st: <http://www.w3.org/ns/qb4st/> | ||
# PREFIX qb4st: <http://www.w3.org/ns/qb4st/> | ||
PREFIX epsg: <http://www.opengis.net/def/crs/EPSG/0/> | ||
PREFIX dt: <http://linked.data.gov.au/def/datatype/> | ||
SELECT <SELECTS> | ||
WHERE { | ||
{ | ||
{ |
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.
Note this bracket might not be closed and might need commenting
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.
Which bracket? there is nothing in the change.
functions.py
Outdated
rdf:predicate geox:transitiveSfOverlap; | ||
rdf:object ?o . | ||
} UNION { | ||
# ?s1 rdf:subject <URI> ; |
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.
I don't think we can take this out until we've rebuilt the linksets and that is beyond the scope of the LDAPI refactors currently underway. AFAIK we use geox:transitiveSfOverlap still and can't depend on geo:sfOverlaps yet
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.
Lets verify.
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.
so in tests with excelerator use cases, we either have to:
a) change the linksets and/or triples in the cache so that this API endpoint works with your proposed query @dr-shorthair
or
b) defer removing the union statement (i.e. keep it) so it works with the current linkset triples
b) is more expedient; but a) is probably what we should do in the longer term
functions.py
Outdated
rdf:predicate geox:transitiveSfOverlap; | ||
rdf:object ?o . | ||
} UNION { | ||
# ?s1 rdf:subject <URI> ; |
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.
Sorry it is the bracket above this comment I think it also needs commenting. Not sure though it just looks like an extra line needs commenting.
rdf:predicate rdf:type ; | ||
rdf:object prov:Location . | ||
} . | ||
# UNION |
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.
Yes I agree it shouldn't be a feature but I can't remember all the reasoning around it from our older discussions. I believe we can actually get rid of overlaps objects altogether because all that information should be elsewhere but @ashleysommer has more history and probably more insight on this. We need to break up scope here as well. It would be good to merge whatever is the minimum set of changes to support the new LDAPIs and deal with larger refactors, like overlaps, later.
So far I have only looked at one file: |
@dr-shorthair i think all the SPARQL in the loci-integration-api is in |
I've simplified the |
# Conflicts: # functions.py
PREFIX epsg: <http://www.opengis.net/def/crs/EPSG/0/> | ||
PREFIX dt: <http://linked.data.gov.au/def/datatype/> | ||
SELECT <SELECTS> | ||
WHERE { | ||
?p rdfs:subPropertyOf* geo:sfOverlaps . |
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 would work, except we haven't loaded the geox ontology in the cache yet.
question is should the cache load the geox ontology also?
geox ontology here: https://github.com/CSIRO-enviro-informatics/geosparql-ext-ont/blob/master/geox-adds.ttl
Yes - it is a very small ontology. But we'll have to make sure that the deprecated elements are back in there. I'll let you know when that's done. |
Shoudl be good now. |
Suppress references to unused namespaces and deprecated predicates