Skip to content

Commit efa6d43

Browse files
fix(clients): keep Python instance references out of numeric values
InstanceRef is no longer an int subclass, so Vector, Quantity, Array and TensorQuantity dimensions and typed.as_int/as_float/as_complex refuse it instead of encoding it as int_value; Connection still sends it as instance_id. Co-Authored-By: jason.han <hanhuijun@gmail.com>
1 parent ddca0ab commit efa6d43

5 files changed

Lines changed: 46 additions & 22 deletions

File tree

clients/python/opensysml/connection.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1722,7 +1722,7 @@ def _python_to_value(self, py_value):
17221722
if isinstance(py_value, bool):
17231723
return sysml_pb2.Value(bool_value=py_value)
17241724
elif isinstance(py_value, InstanceRef):
1725-
return sysml_pb2.Value(instance_id=int(py_value))
1725+
return sysml_pb2.Value(instance_id=py_value.id)
17261726
elif isinstance(py_value, int):
17271727
return sysml_pb2.Value(int_value=py_value)
17281728
elif isinstance(py_value, float):

clients/python/opensysml/values.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -781,31 +781,31 @@ def same_value(a: Any, b: Any) -> bool:
781781
"""Whether two decoded values are the same value, as :class:`SetValue` membership judges it.
782782
783783
``==`` decides, except that a ``bool`` is never a number — ``True`` and ``1``
784-
are distinct values in a model — and an :class:`InstanceRef` is never an
785-
Integer, in a nested ``list`` or :class:`Array` too.
784+
are distinct values in a model — in a nested ``list`` or :class:`Array` too.
786785
"""
787786
if isinstance(a, bool) or isinstance(b, bool):
788787
return isinstance(a, bool) and isinstance(b, bool) and a == b
789-
if isinstance(a, InstanceRef) or isinstance(b, InstanceRef):
790-
return isinstance(a, InstanceRef) and isinstance(b, InstanceRef) and a == b
791788
if isinstance(a, list) and isinstance(b, list):
792789
return len(a) == len(b) and all(same_value(x, y) for x, y in zip(a, b))
793790
return a == b
794791

795792

796-
class InstanceRef(int):
793+
@dataclass(frozen=True)
794+
class InstanceRef:
797795
"""A reference to an instance the client has no instance graph to resolve.
798796
799-
It is the instance's integer id, so it reads and compares as one where an
800-
``int`` is expected; but it is not the Integer of that value in a model, so
801-
:func:`same_value` keeps the two apart, and sent back it is again an
802-
instance reference.
797+
It holds the instance's id and is nothing else: not the Integer of that
798+
value, so it is never equal to one nor accepted where a number is, and sent
799+
back it is again an instance reference.
800+
801+
Attributes:
802+
id (int): The instance's id
803803
"""
804804

805-
__slots__ = ()
805+
id: int
806806

807-
def __repr__(self) -> str:
808-
return f"InstanceRef({int(self)})"
807+
def __str__(self) -> str:
808+
return f"instance({self.id})"
809809

810810

811811
@dataclass(frozen=True)
@@ -941,7 +941,7 @@ def value_to_python(pb_value, resolve_instance=None):
941941
pb_value: sysml_pb2.Value message
942942
resolve_instance: optional callable mapping an instance id to an object;
943943
when omitted, instance references are returned as an
944-
:class:`InstanceRef`, an ``int`` holding the id.
944+
:class:`InstanceRef` holding the id.
945945
946946
Returns:
947947
int, float, complex, bool, str, list, None, :data:`UNSET`, a

clients/python/tests/test_instance.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from opensysml.errors import FeatureValueError
44
from opensysml.proto import sysml_pb2
55
from opensysml.instance import Instance
6+
from opensysml.values import InstanceRef
67

78

89
def scalar_feature(name, **value_kwargs):
@@ -156,16 +157,17 @@ def test_nested_instance_resolution():
156157
assert inst.engine is engine
157158

158159

159-
def test_unresolvable_instance_id_falls_back_to_id():
160-
"""Without the child in the graph, the bare id is returned."""
160+
def test_unresolvable_instance_id_falls_back_to_a_reference():
161+
"""Without the child in the graph, a reference holding the id is returned."""
161162
pb_inst = sysml_pb2.Instance(
162163
id=1,
163164
type_symbol_id="Test::P",
164165
feature_values={"engine": scalar_feature("engine", instance_id=42)},
165166
)
166167

167168
inst = Instance(pb_inst)
168-
assert inst.engine == 42
169+
assert inst.engine == InstanceRef(42)
170+
assert inst.engine != 42
169171

170172

171173
def test_error_slot_raises_slot_error():

clients/python/tests/test_set_tensor.py

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
CAPABILITY_VERIFICATION,
2222
MissingCapabilityError,
2323
)
24+
from opensysml import typed
2425
from opensysml.connection import Connection
2526
from opensysml.enumeration import EnumLiteral
26-
from opensysml.errors import ExecutionError, UnsupportedValueError
27+
from opensysml.errors import ExecutionError, TypeMismatchError, UnsupportedValueError
2728
from opensysml.proto import sysml_pb2
2829
from opensysml.values import (
2930
Array,
@@ -34,6 +35,7 @@
3435
TensorQuantity,
3536
Unit,
3637
UnitFactor,
38+
Vector,
3739
VectorQuantity,
3840
value_to_python,
3941
)
@@ -315,9 +317,10 @@ def test_members_that_only_look_alike_are_distinct(elements, expected):
315317
assert 1 not in SetValue((True,)) and True not in SetValue((1,))
316318

317319

318-
def test_an_unresolved_instance_reference_is_its_id_but_not_an_integer():
320+
def test_an_unresolved_instance_reference_holds_its_id_but_is_not_an_integer():
319321
ref = value_to_python(pb_instance(7))
320-
assert isinstance(ref, InstanceRef) and ref == 7 and repr(ref) == "InstanceRef(7)"
322+
assert isinstance(ref, InstanceRef) and ref.id == 7 and ref == InstanceRef(7)
323+
assert ref != 7 and not isinstance(ref, int) and str(ref) == "instance(7)"
321324
assert ref in SetValue((InstanceRef(7),)) and ref not in SetValue((7,))
322325
assert Array((1,), (ref,)) != Array((1,), (7,))
323326
assert Array((2,), (True, 1)) != Array((2,), (1, 1))
@@ -327,6 +330,25 @@ def test_an_unresolved_instance_reference_is_its_id_but_not_an_integer():
327330
sent = conn._python_to_value(SetValue((ref, 7)))
328331
assert [e.WhichOneof("kind") for e in sent.set.elements] == ["instance_id", "int_value"]
329332
assert sent.set.elements[0].instance_id == 7
333+
assert value_to_python(sent) == SetValue((InstanceRef(7), 7))
334+
335+
336+
def test_an_instance_reference_is_refused_where_a_number_is_meant():
337+
ref = InstanceRef(7)
338+
with pytest.raises(ValueError, match="not a number"):
339+
Vector((1, ref))
340+
with pytest.raises(ValueError, match="not a positive integer"):
341+
Array((ref,), (1,))
342+
with pytest.raises(ValueError, match="not a positive integer"):
343+
TensorQuantity((ref,), [Quantity(1.0, PASCAL)])
344+
with pytest.raises(TypeError):
345+
Quantity(1.0, PASCAL) * ref
346+
with pytest.raises(TypeMismatchError):
347+
typed.as_int("n", ref)
348+
with pytest.raises(TypeMismatchError):
349+
typed.as_float("x", ref)
350+
with pytest.raises(TypeMismatchError):
351+
typed.as_complex("z", ref)
330352

331353

332354
def test_a_set_survives_the_wire_bytes():

clients/python/tests/test_unset.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from opensysml.errors import FeatureValueError
1010
from opensysml.instance import Instance
1111
from opensysml.proto import sysml_pb2
12-
from opensysml.values import UNSET, UnsetType, feature_value_to_python, value_to_python
12+
from opensysml.values import UNSET, InstanceRef, UnsetType, feature_value_to_python, value_to_python
1313

1414
import pytest
1515

@@ -57,7 +57,7 @@ def test_a_valued_slot_and_an_object_valued_one_are_unaffected():
5757
object_valued = sysml_pb2.FeatureValue(
5858
feature_name="engine", value=sysml_pb2.Value(instance_id=7), materialized=True
5959
)
60-
assert feature_value_to_python("engine", object_valued) == 7
60+
assert feature_value_to_python("engine", object_valued) == InstanceRef(7)
6161

6262

6363
def test_an_unmaterialized_slot_is_still_an_error():

0 commit comments

Comments
 (0)