Skip to content

model.base: Copy Qualifier and Extension data in update_nss_from #624

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.

Sign up for GitHub

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

Open
sankalpsthakur wants to merge 5 commits into
base: develop
Choose a base branch
from
+568 −10
Open

model.base: Copy Qualifier and Extension data in update_nss_from #624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e15d4a5
model.base: Copy Qualifier and Extension data in update_nss_from
sankalpsthakur Sep 5, 2026
0b16e07
model.base: Copy name and type in Qualifier/Extension update_from
sankalpsthakur Oct 8, 2026
b0eb4d8
model.base: Only guard the lookup in update_nss_from
sankalpsthakur Oct 22, 2026
b0a0cf7
model.base: Respect AASd-118 in Qualifier/Extension update_from
sankalpsthakur Oct 22, 2026
2a37c9f
model.base: Do not count a Qualifier or Extension as its own duplicate
sankalpsthakur Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 61 additions & 9 deletions sdk/basyx/aas/model/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,17 @@ def supplemental_semantic_id(self) -> ConstrainedList[Reference]:
def supplemental_semantic_id(self, supplemental_semantic_id: Iterable[Reference]):
self._supplemental_semantic_id[:] = supplemental_semantic_id

def _update_semantic_ids_from(self, other: "HasSemantics") -> None:
"""
Copy the ``semantic_id`` and the ``supplemental_semantic_id`` of another object.

The old supplemental semantic IDs are cleared before the semantic ID is replaced, as the semantic ID must not
be removed while there are supplemental semantic IDs (Constraint AASd-118). The new ones are set last.
"""
self.supplemental_semantic_id.clear()
self.semantic_id = other.semantic_id
self.supplemental_semantic_id = other.supplemental_semantic_id


class Extension(HasSemantics):
"""
Expand Down Expand Up @@ -1759,8 +1770,10 @@ def name(self):
def name(self, name: NameType) -> None:
_string_constraints.check_name_type(name)
if self.parent is not None:
if name == self._name:
return
for set_ in self.parent.namespace_element_sets:
if set_.contains_id("name", name):
if set_.contains_id("name", name) and set_.get("name", name) is not self:
raise KeyError(
"Object with name '{}' is already present in the parent Namespace".format(
name
Expand All @@ -1777,6 +1790,22 @@ def name(self, name: NameType) -> None:
# Redundant to the line above. However, this way, we make sure that we really update the _name
self._name = name

def update_from(self, other: "Extension") -> None:

@zrgt zrgt Oct 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Extension.update_from and Qualifier.update_from share four verbatim lines (value_type, value, semantic_id, supplemental_semantic_id), identical comments included. The AASd-118 ordering fix has to be applied in both places and will drift.

HasSemantics is their common base — a HasSemantics.update_from copying semantic_id/supplemental_semantic_id, overridden to add the subtype-specific fields, removes the duplication and also covers other HasSemantics members of NamespaceSets, which get no implementation at all today.

Generated by Claude Code

"""
Internal function to update the object's attributes from a different version of the exact same object.

This function should not be used directly. ``NamespaceSet.update_nss_from()`` calls it for every object that is
present in both NamespaceSets.

:param other: The object to update from
"""
self.name = other.name
self._update_semantic_ids_from(other)
# Assign value_type first so the incoming value is cast against the new type.
self.value_type = other.value_type
Comment thread
sankalpsthakur marked this conversation as resolved.
self.value = other.value
self.refers_to = set(other.refers_to)


class HasKind(metaclass=abc.ABCMeta):
"""
Expand Down Expand Up @@ -1915,8 +1944,10 @@ def type(self):
def type(self, type_: QualifierType) -> None:
_string_constraints.check_qualifier_type(type_)
if self.parent is not None:
if type_ == self._type:
return
for set_ in self.parent.namespace_element_sets:
if set_.contains_id("type", type_):
if set_.contains_id("type", type_) and set_.get("type", type_) is not self:
raise KeyError(
"Object with type '{}' is already present in the parent Namespace".format(
type_
Expand All @@ -1933,6 +1964,23 @@ def type(self, type_: QualifierType) -> None:
# Redundant to the line above. However, this way, we make sure that we really update the _type
self._type = type_

def update_from(self, other: "Qualifier") -> None:
"""
Internal function to update the object's attributes from a different version of the exact same object.

This function should not be used directly. ``NamespaceSet.update_nss_from()`` calls it for every object that is
present in both NamespaceSets.

:param other: The object to update from
"""
self.type = other.type
self._update_semantic_ids_from(other)
# Assign value_type first so the incoming value is cast against the new type.
self.value_type = other.value_type
Comment thread
sankalpsthakur marked this conversation as resolved.
self.value = other.value
self.value_id = other.value_id
self.kind = other.kind


@_string_constraints.constrain_value_type_iec61360("value")
class ValueReferencePair:
Expand Down Expand Up @@ -2367,7 +2415,10 @@ def get(
attribute_value if case_sensitive else attribute_value.upper(), default
)

# Todo: Implement function including tests
# Todo: The removal of objects that are missing in `other` below has known defects. An object is queued once per
# attribute that both sets index, and the second removal raises a KeyError. The lookup in `other` ignores the
# case sensitivity of `other`, attribute names are compared with `is` and an object that is falsy counts as
# missing. Also, objects that are neither Referable, Qualifier nor Extension raise a TypeError.
def update_nss_from(self, other: "NamespaceSet"):
"""
Update a NamespaceSet from a given NamespaceSet.
Expand All @@ -2382,33 +2433,34 @@ def update_nss_from(self, other: "NamespaceSet"):
try:
if isinstance(other_object, Referable):
backend, case_sensitive = self._backend["id_short"]
referable = backend[
existing_object = backend[
other_object.id_short
if case_sensitive
else other_object.id_short.upper()
]
referable.update_from(other_object) # type: ignore
elif isinstance(other_object, Qualifier):
backend, case_sensitive = self._backend["type"]
qualifier = backend[ # noqa: F841 qualifier currently unused
existing_object = backend[
other_object.type
if case_sensitive
else other_object.type.upper()
]
# qualifier.update_from(other_object) # TODO: What should happend here? Remove noqa when done
elif isinstance(other_object, Extension):
backend, case_sensitive = self._backend["name"]
extension = backend[ # noqa: F841 extension currently unused
existing_object = backend[
other_object.name
if case_sensitive
else other_object.name.upper()
]
# extension.update_from(other_object) # TODO: What should happend here? Remove noqa when done
else:
raise TypeError("Type not implemented")

@zrgt zrgt Oct 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_NSO is bound to Union[Referable, Qualifier, HasSemantics, Extension] (line 571), and UniqueSemanticIdNamespace._get_object(HasSemantics, "semantic_id", ...) treats sets as holding bare HasSemantics — so this raises TypeError("Type not implemented") for a legitimate member type, aborting Referable.update_from on its owner.

Not introduced by this PR, but the removed # Todo: Implement function including tests marker was covering it. Either handle the generic HasSemantics case or narrow _NSO.

Generated by Claude Code

except KeyError:

@zrgt zrgt Oct 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Enabling update_from() here puts it inside this try, so a KeyError raised inside update_from is now misread as "object is not in NamespaceSet" — and the element gets destroyed.

class Q(model.Qualifiable):
    def __init__(self, values=()):
        super().__init__()
        self.set1 = model.NamespaceSet(self, [("type", False)], values)

a = Q([model.Qualifier("type1", model.datatypes.Int, 1)])
b = Q([model.Qualifier("TYPE1", model.datatypes.Int, 99)])
a.set1.update_nss_from(b.set1)

The case-insensitive lookup matches, update_from runs self.type = other.type, and the type setter raises KeyError("Object with type 'TYPE1' is already present in the parent Namespace"). It is swallowed, the object lands in objects_to_add, other.remove(obj) succeeds and self.add(obj) raises:

AASConstraintViolation: Object with attribute (name='type', value='TYPE1') is already present in this set of objects (Constraint AASd-021)
a.set1 = [('type1', 1)]   # stale
b.set1 = []               # qualifier gone

On develop the same input is a clean no-op. The try should cover only the backend lookup — use backend.get(...) and call update_from() outside it. Same applies to the pre-existing referable.update_from() path.

Generated by Claude Code

# other object is not in NamespaceSet
objects_to_add.append(other_object)
continue
# Only the lookup above tells that an object is missing. A KeyError raised by update_from() must not be
# mistaken for that, as the object would be removed from `other` and added to this set a second time.
existing_object.update_from(other_object) # type: ignore
for attr_name, (backend, case_sensitive) in self._backend.items():
for attr_name_other, (
backend_other,
Expand Down
Loading