Skip to content
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

Fixes for subclasses and subproperties #303

Merged
merged 6 commits into from Nov 7, 2016
Merged

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Nov 2, 2016

This pull request contains two fixes:

In SALSAH, allow a dependent resource to belong to a subclass

In SALSAH, when you create a new resource A that can contain a link to another resource B, you have the option of creating B first, then A. This pull request fixes the form for creating B, to allow B to belong to a subclass of the class that's required by the link property in A.

For example, if when you're creating an anything:Thing, and you select "New entry..." for the property hasOtherThing, you now get a pull-down menu allowing you to create an anything:Thing or an anything:BlueThing.

Remove property predicate inheritance

So far, Knora has implemented inheritance of the predicates of properties, but there were some problems with this, as discussed in issue #302. After thinking this through, I've come to the conclusion that property predicate inheritance wasn't a good idea to begin with. Consistency checking on GraphDB can't check it, because consistency checking is based on RDFS inference, in which property predicates are not inherited.

I think the original reason for this confusion was that resource classes inherit cardinalities, because cardinalities are defined using rdfs:subClassOf, so with RDFS inference, a resource class inherits the cardinalities of its base classes. So by analogy, we thought that properties should inherit the predicates of their base properties. But properties don't have cardinalities for their predicates, so there is actually nothing like this for properties in RDFS semantics.

As we found out in #302, we couldn't actually check property predicate inheritance anywhere except in Scala: neither the WHERE clauses of SPARQL updates, nor the GraphDB consistency checker, could check it. So it was hard to enforce in addition to being nonstandard.

Therefore I think the safest and clearest thing to do is remove it, and require all property predicates to be defined explicitly on each property.

To ensure that nobody forgets to specify knora-base:objectClassConstraint, which is required on all subproperties of knora-base:hasValue or knora-base:hasLinkTo (as stated in the Knora base ontology doc), I've added a consistency rule to check that it's provided.

Closes #302.

@benjamingeer benjamingeer added the bug something isn't working label Nov 2, 2016
@benjamingeer benjamingeer added this to the Beta Release milestone Nov 2, 2016
@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Nov 2, 2016

Since you are working on linking properties, could you have a look at this: #277?

@tobiasschweizer
Copy link
Contributor

tobiasschweizer commented Nov 2, 2016

If the user forgets to fill in the label field, an error is thrown (in jquery.resadd.js:1140):

TypeError: tmplabel is undefined
var tmplabelFirstElem = tmplabel[0];

A check should be added if tmplabel !== undefined and tmplabel.length > 0 and a message should be printed out to the user if the check failed, telling him to enter a label.

…ty has a narrower objectClassConstraint than its base property.
@benjamingeer
Copy link
Author

In addition to fixing the above, I'm going to take care of #302 on this branch, by removing all property predicate inheritance from the ontology responder, and requiring each property to define its own predicates explicitly.

@tobiasschweizer
Copy link
Contributor

Does this mean that you need more time? That's fine for me. I could have another look at his pull request next week.

@benjamingeer
Copy link
Author

Yes, I'm going to try to finish this today, then you can look at it next week.

@benjamingeer benjamingeer changed the title fix (salsah): Allow a dependent resource to belong to a subclass Fixes for subclasses and subproperties Nov 4, 2016
@benjamingeer
Copy link
Author

OK, I think this is done. I fixed the problem if the user forgets to fill in the label. But I don't think I can do #277 without Lukas's help, so let's do that in another pull request. You can review this again now.

knora-base:subjectClassConstraint :Thing ;

knora-base:objectClassConstraint :BlueThing ;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

salsah-gui:guiElement salsah-gui:Searchbox ; has to be added here since you removed property predicate inheritance. Also permissions are missing.

@@ -308,6 +308,13 @@ Id: maxCardinality_1_table
* Consistency rules for subject and object class constraints.
*/

// Every subproperty of knora-base:resourceProperty must have a knora-base:objectClassConstraint.
Consistency: object_class_constraint_required
p <rdfs:subPropertyOf> <knora-base:resourceProperty> [Constraint p != <knora-base:resourceProperty>]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a similar check for other property predicates such as GUI element? If such information is missing, the GUI does not work correctly.

@tobiasschweizer
Copy link
Contributor

The check for an empty label works fine!

@benjamingeer
Copy link
Author

Could we add a similar check for other property predicates such as GUI element? If such information is missing, the GUI does not work correctly.

Not everyone wants to use the SALSAH GUI, or any GUI. And if they make their own GUI, they may not need these predicates, or they may prefer to make their own predicates for their GUI.

@tobiasschweizer
Copy link
Contributor

Ok, but you still have to add the gui element in the anything ontology

- Add missing predicates in anything-onto.ttl.
SepidehAlassi added a commit that referenced this pull request Dec 13, 2016
* develop:
  Improve configuration for more concurrent requests (#338)
  Upgrade to Scala 2.12 (#343)
  Fix various inconsistencies in knora-base.ttl and docs. (#330)
  Fix language switching and login/logout in SALSAH (#331)
  Graph data API operation (#267)
  Allow connections over HTTPS (#332)
  Update README.md
  Add link to GraphDB.
  fix (salsah): Add missing JavaScript “var”.
  fix (webapi): Fix RouteUtilV1 so it returns HTML instead of JSON when requested (#326)
  Ontology and documentation fixes. (#325)
  fix (salsah): Create multiple links in a resource, and other bug fixes (#315)
  feature (webapi): Make TransformData fix “test” labels on regions. (#311)
  Fixes for subclasses and subproperties (#303)
  Add script to update docs on knora.org (#300)
  docs: Make trivial change for testing purposes.
  fix (webapi): CORS problems (#307)
  Use inference to optimise searches on GraphDB (#301)
@benjamingeer benjamingeer mentioned this pull request Jul 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Property inheritance: Knora-base:objectClassConstraint & Knora-base:subjectClassConstraint?
3 participants