Set id_short of Identifiables to optional
#139
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Constraint AASd-120 requires direct children of a `SubmodelElementList` to have id_short=None. On the contrary, `SubmodelElementList` must be a Namespace, since children of Lists must still be referable via References, and also must be allowed to reference their parent, which is expected to be a Namespace. Since id_short=None must hold for all direct children, they lack a unique identifying attribute, that can be used to refer to an item. However, this is required for a Namespace. Thus, we had two options for implementing this: - Refactor a lot of the model.base module such that `SubmodelElementLists` are considered Namespaces - Generate a unique id_short for every direct children of a `SubmodelElementList` whenever it is added. Since the first alternative would require a distinction for `SubmodelElementList` in all places where a `Namespace` is used, we decided on the second alternative. This commit implements the generation of unique id_shorts via the `item_id_set_hook`, that was recently added to `NamespaceSet` and `OrderedNamespaceSet`. It is called for every added SubmodelElement. Furthermore, the `item_id_del_hook` is called for every removed SubmodelElement and used to remove the generated id_short again. This aside, the examples and unit tests are also adjusted such that the id_short is removed for all direct children of `SubmodelElementList`. Furthermore, a test for `AASd-120` is added. The AASDataChecker is adjusted to skip the comparison of id_short for direct children of `SubmodelElementList`, since these are generated and thus never the same now. For the same reason, the XML/JSON serialisation is adjusted to skip serialising the id_short if direct children of a `SubmodelElementList`.
…ntList` Since direct children of `SubmodelElementList` don't have an identifying attribute anymore (AASd-120), they cannot be compared because it is impossible to know which SubmodelElement should be compared against which other element. Maybe this can be implemented again in the future, when hashing is implemented for all SubmodelElements, but for now we raise a `NotImplementedError`. A test-case for this behavior is added and `order_relevant` is set to `true` in all example files.
This commit applies the following changes to all test-files: - The id_short of direct children of a `SubmodelElementList` is removed - `SubmodelElementList.order_relevant` is set to true for all `SubmodelElementList`s
This commit extends eclipse-basyx@bffb075.
…ated_id_shorts_aasd_120' into fix/id_short
As default value of Submodel.id_short, ConceptDescription.id_short, AAS.id_short is set to None, test files had to be updated.
s-heppner
added a commit
that referenced
this pull request
Oct 19, 2023
In order to be able to merge #139, we need to manually adapt the aasx test files. This commit is yet another reason for why we should stop using binary test files.
|
Looks good to me. However I really started to dislike the binary aasx test files. |
Frosty2500
pushed a commit
to rwth-iat/basyx-python-sdk
that referenced
this pull request
Aug 22, 2024
In order to be able to merge eclipse-basyx#139, we need to manually adapt the aasx test files. This commit is yet another reason for why we should stop using binary test files.
Frosty2500
pushed a commit
to rwth-iat/basyx-python-sdk
that referenced
this pull request
Aug 22, 2024
Set `id_short` of `Identifiables` to optional
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit extends bffb075 and fixes #138
This PR depends on #133