-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
implement constraint AASd-120 #133
Merged
s-heppner
merged 3 commits into
eclipse-basyx:improve/V30
from
rwth-iat:feature/SubmodelElementList_generated_id_shorts_aasd_120
Oct 18, 2023
Merged
implement constraint AASd-120 #133
s-heppner
merged 3 commits into
eclipse-basyx:improve/V30
from
rwth-iat:feature/SubmodelElementList_generated_id_shorts_aasd_120
Oct 18, 2023
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
jkhsjdhjs
force-pushed
the
feature/SubmodelElementList_generated_id_shorts_aasd_120
branch
from
October 12, 2023 16:28
bcd949f
to
b44c904
Compare
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
jkhsjdhjs
force-pushed
the
feature/SubmodelElementList_generated_id_shorts_aasd_120
branch
from
October 12, 2023 16:45
b44c904
to
a0a2416
Compare
Wow. Thank you for conquering this monstrosity! It looks good to me, so I can rebase it when you're ready. |
Yep 👍 |
s-heppner
deleted the
feature/SubmodelElementList_generated_id_shorts_aasd_120
branch
October 18, 2023 11:18
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.
generate unique id_shorts for SubmodelElementList-children
Constraint AASd-120 requires direct children of a
SubmodelElementList
to have id_short=None. On the contrary,
SubmodelElementList
must be aNamespace, 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:
SubmodelElementLists
are considered Namespaces
SubmodelElementList
whenever it is added.Since the first alternative would require a distinction for
SubmodelElementList
in all places where aNamespace
is used, wedecided on the second alternative.
This commit implements the generation of unique id_shorts via the
item_id_set_hook
, that was recently added toNamespaceSet
andOrderedNamespaceSet
. It is called for every added SubmodelElement.Furthermore, the
item_id_del_hook
is called for every removedSubmodelElement 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 andthus 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
.examples.data._helper: disable comparison of unordered
SubmodelElementList
Since direct children of
SubmodelElementList
don't have an identifyingattribute 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 totrue
in all example files.test: update compliance tool test-files
This commit applies the following changes to all test-files:
SubmodelElementList
is removedSubmodelElementList.order_relevant
is set to true for allSubmodelElementList
sThis PR depends on #132