Skip to content

fix: include elem_type in creation of notion with parent#4

Merged
felixocker merged 2 commits intofelixocker:mainfrom
robert-mieth:fix-create-notion
Dec 23, 2022
Merged

fix: include elem_type in creation of notion with parent#4
felixocker merged 2 commits intofelixocker:mainfrom
robert-mieth:fix-create-notion

Conversation

@robert-mieth
Copy link
Contributor

It seems reasonable to also include the elem_type in the creation of new notions with parents.

I came across this when using add_instances(), as the use of a DataProperty created as a child of another DataProperty leads to erroneous behavior. Line 700 (if DataProperty in pred.is_a:) checks against the elem_type, which is currently only included for notions without parents. This seems to also apply to line 710. Predicates, that are created as children of other predicates, simply lack the DataProperty or ObjectProperty parent, which is required to evaluate the conditions (l.700, l.710) to true.

Here is a minimal example:

base_classes = [["foo", None],]
base_dps = [["has_value", None, True, None, None, None, None, None, None, None],
            ["has_string_value", "has_value", True, None, None, None, None, None, None, None],]
base_instances = [["bar", "foo", "has_string_value", "baz", "string"],]

Adding the instance currently results in pred.is_a [base.has_value, owl.FunctionalProperty] instead of the required [base.has_value, owl.DatatypeProperty, owl.FunctionalProperty].

@robert-mieth
Copy link
Contributor Author

Oops, I just noticed that my suggested fix should probably only be applied to ObjectProperties and DataProperties and not classes to maintain a clean class hierarchy. I'll try to adapt my PR accordingly.

@felixocker felixocker merged commit 8bdb120 into felixocker:main Dec 23, 2022
@felixocker
Copy link
Owner

thanks!

felixocker added a commit that referenced this pull request Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants