Skip to content

Keywords with context input#513

Merged
jesper-friis merged 23 commits intomasterfrom
keywords-with-context-input
Mar 12, 2026
Merged

Keywords with context input#513
jesper-friis merged 23 commits intomasterfrom
keywords-with-context-input

Conversation

@jesper-friis
Copy link
Copy Markdown
Contributor

@jesper-friis jesper-friis commented Mar 9, 2026

Description

Added context argument to get_keywords().

Some functionality that should have been in PR #513 has leaked into this PR

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?

@jesper-friis jesper-friis mentioned this pull request Mar 9, 2026
13 tasks
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 9, 2026

Codecov Report

❌ Patch coverage is 97.67442% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.01%. Comparing base (b793b49) to head (e307f5c).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
tripper/datadoc/dataset.py 96.15% 1 Missing ⚠️
tripper/datadoc/keywords.py 97.36% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should have som check on what happens if there is mismatch between previously added context and updated_context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not prioritised for now. Added a TODO in test_dataset.py

DCTERMS.creator: "some",
DCTERMS.hasPart: "value",
DCTERMS.issued: "value",
# DCTERMS.issued: "value",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Comment on lines +616 to +622
{
# 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"],
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For completeness we should have individual relating to one individual and individual related to a list of individuals

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a TODO about this in test_dataset.py. Will be addressed in the next PR that focuses on the update_restrictions() funtions.

Comment on lines +669 to +670
# WRONG! Should be converted to restrictions
"@id": "ex:instr3",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be fixed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, working on this in PR #514

"skos:prefLabel",
RDFS.label,
"rdfs:label",
"prefLabel",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suddenly prefLabel is secondary in precendense.
All these alternatives should be tested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. The order is by purpose. Added a comment about in in the code.

@jesper-friis jesper-friis merged commit 985b49e into master Mar 12, 2026
22 checks passed
@jesper-friis jesper-friis deleted the keywords-with-context-input branch March 12, 2026 07:51
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