sdk: Fix walk_submodel() skipping Entity and Operation children#465
sdk: Fix walk_submodel() skipping Entity and Operation children#465zrgt wants to merge 1 commit intoeclipse-basyx:developfrom
Conversation
Fixes eclipse-basyx#423. The traversal now recurses into Entity.statement and Operation.input_variable/output_variable/in_output_variable, so File SubmodelElements nested inside these containers are no longer silently dropped when reading AASX files. Adds test_traversal.py with 13 unit tests covering the fix and regression cases.
| def _walk_submodel_helper(elements: Iterable[model.SubmodelElement]) -> Iterator[model.SubmodelElement]: | ||
| for element in elements: | ||
| if isinstance(element, (model.SubmodelElementCollection, model.SubmodelElementList)): | ||
| yield from _walk_submodel_helper(element.value) | ||
| elif isinstance(element, model.Operation): | ||
| for var_list in (element.input_variable, element.output_variable, element.in_output_variable): | ||
| yield from _walk_submodel_helper(var_list) | ||
| elif isinstance(element, model.Entity): | ||
| yield from _walk_submodel_helper(element.statement) | ||
| yield element |
There was a problem hiding this comment.
I would keep this code in the walk_submodel() method.
I don't really see a use case for reusal, which would outweigh the annoyance to jump from one method to the other just to understand what's happening.
I'm free to be convinced otherwise though ;)
| def test_collection_post_order(self): | ||
| child1 = model.Property("child1", model.datatypes.String) | ||
| child2 = model.Property("child2", model.datatypes.String) | ||
| coll = model.SubmodelElementCollection("coll", value=[child1, child2]) | ||
| sm = self._submodel(coll) | ||
| result = list(walk_submodel(sm)) | ||
| # post-order: children before parent | ||
| self.assertEqual([child1, child2, coll], result) |
There was a problem hiding this comment.
This was implicitly the ordering before. I'm not sure we should guarantee that ordering, though.
I think the method should only guarantee that all elements are returned, without making any promises about their order, especially since this is a yield-based method.
Therefore, I'd remove the tests regarding order.
Also, I'd reformulate the asserts as asserting just that all the elements are returned, not their exact order.
2 participants
Summary
Fixes #423.
traversal.walk_submodel()was only recursing intoSubmodelElementCollection.valueandSubmodelElementList.value, missing two other container types defined by the AAS specification:Entity.statementOperation.input_variable,Operation.output_variable,Operation.in_output_variableThis caused a concrete data-loss bug:
FileSubmodelElements nested inside anEntityorOperationwere silently dropped from thefile_storewhen reading AASX files(
AASXReader._collect_supplementary_filesuseswalk_submodelto find allFileelements).Changes
sdk/basyx/aas/util/traversal.py: Extracted element-level recursion into a private helper_walk_submodel_helper()that handles all four container types. The post-order traversal(children before parent) is preserved and works correctly for arbitrary nesting
(e.g.
EntityinsideOperation.input_variable,SubmodelElementCollectioninsideEntity.statement).sdk/test/util/test_traversal.py(new): 13 unit tests covering basic traversal,Entitystatements,
Operationvariables, nested combinations, empty containers, and two explicitregression tests for issue traversal.walksubmodel(): Child Submodel Elements of
EntityandOperationare skipped #423.