Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
29 changes: 29 additions & 0 deletions src/spatial_graph/_graph/graph_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,33 @@
SRC_DIR = Path(__file__).parent


def _cpp_member_name(name: str) -> str:
"""C++ struct member name for a user-provided node/edge attribute.

User attribute names are used verbatim as the Python-facing API (property
names, ``get_*``/``set_*`` methods), but they cannot be used directly as
C++ identifiers: a name like ``int16`` collides with the MSVC built-in
``__int16`` (Microsoft extensions treat ``int8``/``int16``/``int32``/
``int64`` *and* their single-underscore forms ``_int8``/... as synonyms for
the ``__intN`` keywords), and others may clash with C++ keywords (``class``,
``new``, ...). We therefore mangle the C++ member with a fixed prefix and
alias it back to the user name on the Cython side, so the public API is
unchanged while the generated C++ always compiles. The prefix matters: a
bare ``_`` prefix would turn ``int8`` into ``_int8``, which is itself an
MSVC keyword, so the prefix must not be ``_`` followed by a digit.
"""
return "_attr_" + name


def _cpp_arg_name(name: str) -> str:
"""C++ constructor parameter name for a user-provided attribute.

Must be distinct from :func:`_cpp_member_name` (the constructor initializes
``member(arg)``) and, like it, must avoid the MSVC ``_intN`` keyword forms.
"""
return "_arg_" + name


def _build_wrapper(
node_dtype: str,
node_attr_dtypes: Mapping[str, str] | None = None,
Expand Down Expand Up @@ -60,6 +87,8 @@ def _build_wrapper(
name: DType(dtype) for name, dtype in edge_attr_dtypes.items()
}
wrapper_template.directed = directed
wrapper_template.cpp_member = _cpp_member_name
wrapper_template.cpp_arg = _cpp_arg_name

return str(wrapper_template)

Expand Down
14 changes: 7 additions & 7 deletions src/spatial_graph/_graph/wrapper_template.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -95,31 +95,31 @@ cdef extern from *:
${class_name}(
%set $sep=""
%for name, dtype in $dtypes.items()
$sep$dtype.to_c_decl("_" + $name)
$sep$dtype.to_c_decl($cpp_arg(name))
%set $sep=", "
%end for
) :
%set $sep=""
%for name, dtype in $dtypes.items()
$sep
%if $dtype.is_array
${name}{
${cpp_member(name)}{
%set $isep=""
%for i in range($dtype.size)
${isep}_${name}[$i]
${isep}${cpp_arg(name)}[$i]
%set $isep=", "
%end for
}
%else
${name}(_$name)%slurp
${cpp_member(name)}(${cpp_arg(name)})%slurp
%end if
%set $sep=", "
%end for
{};
%end if

%for name, dtype in $dtypes.items()
$dtype.to_c_decl($name);
$dtype.to_c_decl($cpp_member(name));
%end for
};
"""
Expand All @@ -129,13 +129,13 @@ cdef extern from *:
${class_name}(
%set $sep=""
%for name, dtype in $dtypes.items()
$sep$dtype.to_c_decl("_" + $name)
$sep$dtype.to_c_decl($cpp_arg(name))
%set $sep=", "
%end for
) except +

%for name, dtype in $dtypes.items()
$dtype.to_pyxtype() $name
$dtype.to_pyxtype() $name "$cpp_member(name)"
%end for

cdef class ${class_name}View:
Expand Down
55 changes: 55 additions & 0 deletions tests/test_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,58 @@ def test_attr_dtypes(dtype):
graph.add_node(1, position=np.array([0.0, 0.0, 0.0]), **{f"node_attr_{dtype}": 0})
graph.add_node(2, position=np.array([0.0, 0.0, 0.0]), **{f"node_attr_{dtype}": 1})
graph.add_edge([1, 2], **{f"edge_attr_{dtype}": 0})


# Attribute names that collide with C++ keywords or MSVC built-in type
# specifiers (but are not Python keywords, so they are valid attribute names).
# These cannot appear verbatim as C++ struct members or constructor parameters.
# In particular MSVC (with Microsoft extensions) treats ``int8``/``int16``/
# ``int32``/``int64`` *and* their single-underscore forms ``_int8``/... as
# aliases for the ``__intN`` keywords, so a member named ``int16`` (or a
# constructor parameter ``_int16``) expands to ``__int16`` and fails to compile.
@pytest.mark.parametrize(
"attr_name", ["int8", "int16", "int32", "int64", "new", "double"]
)
def test_attr_name_collides_with_cpp_keyword(attr_name):
graph = sg.SpatialGraph(
ndims=3,
node_dtype="uint64",
node_attr_dtypes={attr_name: "int16", "position": "double[3]"},
edge_attr_dtypes={attr_name: "int16"},
position_attr="position",
)

graph.add_node(1, position=np.array([1.0, 1.0, 1.0]), **{attr_name: 5})
graph.add_node(2, position=np.array([2.0, 2.0, 2.0]), **{attr_name: 7})
graph.add_edge([1, 2], **{attr_name: 9})

# The user-provided name stays the public attribute (the colliding name only
# affects the internal C++ identifier). Reading it back through every public
# access path must return the stored value -- a regression in the C++ member
# mangling / Cython cname-aliasing would silently corrupt these.
nodes = np.array([1, 2], dtype="uint64")

# single node
assert getattr(graph.node_attrs[1], attr_name) == 5
assert getattr(graph.node_attrs[2], attr_name) == 7
# array of nodes
np.testing.assert_array_equal(
getattr(graph.node_attrs[nodes], attr_name), np.array([5, 7], dtype="int16")
)
# all nodes (order is unspecified, so compare sorted)
np.testing.assert_array_equal(
np.sort(getattr(graph.node_attrs, attr_name)), np.array([5, 7], dtype="int16")
)
# edge
assert getattr(graph.edge_attrs[(1, 2)], attr_name) == 9

# the position attribute is mangled the same way and must be unaffected
np.testing.assert_array_equal(
graph.node_attrs[1].position, np.array([1.0, 1.0, 1.0])
)

# setting through the public attribute round-trips too
setattr(graph.node_attrs[1], attr_name, 11)
assert getattr(graph.node_attrs[1], attr_name) == 11
setattr(graph.edge_attrs[(1, 2)], attr_name, 13)
assert getattr(graph.edge_attrs[(1, 2)], attr_name) == 13
Loading