Conversation
…rds-with-context-input
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #513 +/- ##
==========================================
+ Coverage 84.58% 85.01% +0.43%
==========================================
Files 30 30
Lines 4890 4959 +69
==========================================
+ Hits 4136 4216 +80
+ Misses 754 743 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| c = context.get_context_dict() | ||
| assert c["instr"] == {"@id": EX.instr, "@type": HUME.Device} | ||
| assert c["MyDevice"] == {"@id": EX.MyDevice, "@type": OWL.Class} | ||
| assert c["Device"] == {"@id": HUME.Device, "@type": OWL.Class} |
There was a problem hiding this comment.
Should have som check on what happens if there is mismatch between previously added context and updated_context.
There was a problem hiding this comment.
Not prioritised for now. Added a TODO in test_dataset.py
| DCTERMS.creator: "some", | ||
| DCTERMS.hasPart: "value", | ||
| DCTERMS.issued: "value", | ||
| # DCTERMS.issued: "value", |
There was a problem hiding this comment.
Why is this commented out?
There was a problem hiding this comment.
Because dcterms:issued is a data property which should result in a value restriction. However, since there are no configuration options for value restrictions (except for creating a blank node, which we currently doesn't support), there are no reasons to include value restriction in the data structure returned by infer_restriction_types().
The value restriction for dcterms:hasPart should probably also be removed.
That will be addressed in upcoming PRs.
| { | ||
| # An individial relating to two classes and an individual. | ||
| # Should be converted to an existential restriction. | ||
| "@id": "ex:instr3", | ||
| "@type": HUME.Device, | ||
| "hasPart": [HUME.MeasuringInstrument, "MyDevice", "ex:instr"], | ||
| }, |
There was a problem hiding this comment.
For completeness we should have individual relating to one individual and individual related to a list of individuals
There was a problem hiding this comment.
Added a TODO about this in test_dataset.py. Will be addressed in the next PR that focuses on the update_restrictions() funtions.
| # WRONG! Should be converted to restrictions | ||
| "@id": "ex:instr3", |
| "skos:prefLabel", | ||
| RDFS.label, | ||
| "rdfs:label", | ||
| "prefLabel", |
There was a problem hiding this comment.
Suddenly prefLabel is secondary in precendense.
All these alternatives should be tested.
There was a problem hiding this comment.
Yes. The order is by purpose. Added a comment about in in the code.
# Description Support individuals with restrictions and lists of restrictions. Builds on top of PR #513 This works. Possible further improvements (may as well be done in new PRs): - [ ] Add missing tests suggested in the review of #513 - [ ] Remove infer_restriction_types() and rename the `restrictions` argument of `update_restrictions()` to `spec`. - [ ] Catch `jsonld.JsonLdError` in context.py:210 and raise a more readable exception. ## Type of change - [ ] Bug fix and code cleanup - [ ] New feature - [ ] Documentation update - [ ] Testing ## Checklist for the reviewer This checklist should be used as a help for the reviewer. - [ ] Is the change limited to one issue? - [ ] Does this PR close the issue? - [ ] Is the code easy to read and understand? - [ ] Do all new feature have an accompanying new test? - [ ] Has the documentation been updated as necessary? - [ ] Is the code properly tested?
2 participants
Description
Added
contextargument to get_keywords().Some functionality that should have been in PR #513 has leaked into this PR
Type of change
Checklist for the reviewer
This checklist should be used as a help for the reviewer.