Skip to content

Resolve #383: Test ld_context#403

Merged
sdruskat merged 18 commits intorefactor/data-modelfrom
refactor/383-test-ld_context
Aug 5, 2025
Merged

Resolve #383: Test ld_context#403
sdruskat merged 18 commits intorefactor/data-modelfrom
refactor/383-test-ld_context

Conversation

@sdruskat
Copy link
Copy Markdown
Contributor

@sdruskat sdruskat commented Jul 28, 2025

This PR includes tests for src/hermes/model/types/ld_context.py.

It also includes some FIXMEs to resolve during refactoring (#352).

Finally, it adds missing licenses.

Note: I make some assumptions in the tests. Please check carefully if you agree.

  1. I'm assuming that the context should handle requests for non-existing terms with an exception (to be defined during refactoring).
  2. I'm assuming that __get_item__ should only accept valid input, and raise an exception on invalid input. This is currently testing against Python's TypeError and ValueError which I think is the correct way of handling this.
  3. I'm assuming that even on input that is technically valid (a dict with > 1 items) and semantically valid (dict contains existing keys and values), getting the item fails, as it is unclear what to expect as return. In theory, this input could return a tuple of expanded terms, but how would we handle mixed input of valid/invalid terms, etc.

@sdruskat sdruskat changed the base branch from develop to refactor/data-model July 28, 2025 05:59
@sdruskat sdruskat marked this pull request as ready for review July 28, 2025 06:27
@sdruskat sdruskat added the data model Related to the hermes data model label Jul 28, 2025
@sdruskat sdruskat force-pushed the refactor/383-test-ld_context branch from 76a65e2 to d1f4468 Compare July 28, 2025 06:40
Copy link
Copy Markdown
Member

@led02 led02 left a comment

Choose a reason for hiding this comment

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

Very good try, almost got it right... so no further docs needed? 😈

Clarified some decisions in the review.

Comment thread src/hermes/model/types/ld_context.py Outdated
Comment thread src/hermes/model/types/ld_context.py Outdated
Comment thread test/hermes_test/model/types/test_ld_context.py
@led02
Copy link
Copy Markdown
Member

led02 commented Jul 29, 2025

TIL: The class name maybe should be CURIE ... although the spec seems a bit broken due to the rule "reference := irelative-ref (as defined in IRI)"...

@sdruskat
Copy link
Copy Markdown
Contributor Author

TIL: The class name maybe should be CURIE ... although the spec seems a bit broken due to the rule "reference := irelative-ref (as defined in IRI)"...

Yea, well, no 😅.

I pull the practicality beats purity card ;).

@sdruskat sdruskat requested a review from led02 July 31, 2025 11:09
@sdruskat sdruskat changed the title Resolve #383: Test `ld_context Resolve #383: Test ld_context Aug 5, 2025
@sdruskat sdruskat merged commit 7b10cdc into refactor/data-model Aug 5, 2025
3 of 5 checks passed
@sdruskat sdruskat deleted the refactor/383-test-ld_context branch August 5, 2025 07:47
@led02
Copy link
Copy Markdown
Member

led02 commented Aug 5, 2025

Hmmm... tests don't work.

Okay, I understand that the tests represent the should be API.
But... there were changes to the ContextPrefix class ... renaming the prefix to context, still testing against prefix?

Copy link
Copy Markdown
Member

@led02 led02 left a comment

Choose a reason for hiding this comment

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

While running tests, I stumbled up some more mis-conceptions.


def test_codemeta_prefix(ctx):
"""Default vocabulary in context has the correct base IRI."""
assert ctx.prefix[None] == "https://codemeta.github.io/terms/"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The CodeMeta 2.0 context introduces an codemeta prefix that points to that URI.
However, the default context is still the CodeMeta DOI.

Comment on lines +32 to +33
item = ctx["maintainer"]
assert item == "https://codemeta.github.io/terms/maintainer"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, this test is actually correct but fails.
However, this is not quite unintended...
What you expect the context to do here is to fully expand the IRI (by applying a two-step resolution).
For this purpose, the implementation uses the PyLdProcessor which does a great job.
The ContextPrefix class is meant as a small util that does not implement the full JSON-LD algorithm.

sdruskat added a commit that referenced this pull request Aug 7, 2025
- Remove duplicate tests of __get_item__ on tuples
- This includes test parameters that we decided not to test, as the
API doesn't see them as valid (list inputs), although they're
supported at runtime.
- Related:
#403 (comment)
SKernchen added a commit that referenced this pull request Dec 12, 2025
…ailures

Extend #403: Mark xfailing tests, raise errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data model Related to the hermes data model

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test ld_context

2 participants