Skip to content

Comments

Refactor/381 test ld container#398

Merged
SKernchen merged 33 commits intorefactor/data-modelfrom
refactor/381-test-ld_container
Aug 7, 2025
Merged

Refactor/381 test ld container#398
SKernchen merged 33 commits intorefactor/data-modelfrom
refactor/381-test-ld_container

Conversation

@SKernchen
Copy link
Contributor

Test ld_container

@SKernchen SKernchen self-assigned this Jul 24, 2025
@SKernchen SKernchen added the data model Related to the hermes data model label Jul 24, 2025
@led02
Copy link
Member

led02 commented Jul 26, 2025

Comment on lines 56 to 57
cont_parent = ld_container({"ham": [{"@value": "eggs"}]})
cont = ld_container({"spam": [{"@value": "bacon"}]}, parent=cont_parent)
Copy link
Member

Choose a reason for hiding this comment

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

Construction principle for parent/child containers should be the following (even though, you should not construct ld_container instances due to their abstractness and internal nature ;) )

child_data = [{"spam": [{"@value": "bacon"}]}]
parent_data = [{"ham": child_data}]

cont_parent = ld_container(parent_data)
cont = ld_container(child_data, parent=cont_parent, key="ham")

Important information that is hidden here (and should be explicited in documentation aritfacts):

  • ld_container classes work as proxies and reflect the content and structure of the maintained datasets.
  • All parent-child-relations of ld_containers need to be existent in the base dataset for this to work. (The opposite is not required).
  • All datasets that are maintained by the ld_container are handled by reference! I.e., if you change something in the ld_container, it will be changed in the dataset directly. There is no place where a container creates a copy of the base dataset.
  • If a container has a parent, it is expected to have either index or key (but not both).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That helps a lot, thank you!

@SKernchen SKernchen changed the title Draft: Refactor/381 test ld container Refactor/381 test ld container Jul 31, 2025

def test_to_expanded(self):
cont = ld_container([{"spam": [{"@value": "bacon", "@id": "ham", "@type": "@id"}]}])
cont.active_ctx['_uuid'] = str(uuid.uuid1()) # FIXME: 406
Copy link
Member

Choose a reason for hiding this comment

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

Okay, active_ctx is something internal to pyld and is used here ... as some kind of shortcut. Problem is, that is even for pyld something internal and hence not (very well) documented.

I still see that #406 is an issue, though. Again, a stacktrace would be helpful (but I can also run the tests myself for further analysis).

The other question is, how _to_expanded_json should behave on undefined properties...

@sdruskat sdruskat linked an issue Jul 31, 2025 that may be closed by this pull request
Comment on lines 93 to 115
def test_to_python(self):
cont = ld_container([{"spam": [{"@value": "bacon", "@id": "ham", "@type": ["@id"]}]}])
assert cont._to_python("spam", [{"@value": "bacon"}]) == 'bacon'
assert cont._to_python("@id", "ham") == "ham"

cont.active_ctx['_uuid'] = str(uuid.uuid1()) # FIXME: 406
assert cont._to_python("@type", ["@id"]) == '@id'

def test_to_expanded(self):
# Define a mock vocabulary to work with
mock_context = {
"ham": {"@id": "http://ham.eggs/ham", "@type": "@id"},
"spam": {"@id": "http://ham.eggs/spam"},
"Eggs": {"@id": "http://ham.eggs/Eggs"},
}

# Create container with mock context
cont = ld_container([{}], context=[mock_context])

# Try simple cases of expansion
assert cont._to_expanded_json("spam", "bacon") == [{"@value": "bacon"}]
assert cont._to_expanded_json("@id", "ham") == "http://ham.eggs/ham"
assert cont._to_expanded_json("@type", "@id") == ["@id"]
Copy link
Member

Choose a reason for hiding this comment

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

Okay, testing these two methods will be tough. They basically reflect the 'inner magic' of the type mapping.

This means we should carefully select suitable test cases. Also it is important that the context is valid and reflects the vocabulary used in the test case.

led02 added 12 commits August 1, 2025 20:57
Basically, this is a small but somewhat representative vocabulary that can be used.
In addition, there are two representations of the same document (compact, expanded).
cont.active_ctx is very special with regards to the implementation of a dependency.
This is ugly and tests will be added to test_pyld_utils.
The conversion tests (`_to_python` and `_to_expanded_json`) are pretty long now...
@led02
Copy link
Member

led02 commented Aug 1, 2025

Finally, the expected test failures show up. These need in fact to be fixed (see #412).

Fixing this in refactor/data-model and merging it back to profit from other updates, too.

led02 added 3 commits August 5, 2025 22:32
@SKernchen SKernchen merged commit 2969b09 into refactor/data-model Aug 7, 2025
1 of 5 checks passed
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_container

2 participants