Skip to content

Conversation

@junikimm717
Copy link
Collaborator

Turn immutable tuples into C structs, and keep mutable tuples as C pointers.

Turn immutable tuples into C structs, and keep mutable tuples as C
pointers.
@junikimm717
Copy link
Collaborator Author

While doing this PR, I have been stomping out other bugs, including stuff from my own codegen tests
Did you know that ctypes conversions of numpy scalars all share the same buffer?

import numpy as np
import numpy.ctypeslib as ctl

a = np.int64(2)
b = np.int64(1)

s1 = ctl.as_ctypes(a)
s2 = ctl.as_ctypes(b)

print(s1.value, s2.value)
# this prints 1 1 :skull:

Copy link
Member

@willow-ahrens willow-ahrens left a comment

Choose a reason for hiding this comment

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

Looks good, I would only ask for an ImmutableFType class-based interface.

Even though literally everything should be immutable right now

def scalar_to_ctypes_copy(fmt, obj):
"""
This hack is required because it turns out that scalars don't own memory or smth
Copy link
Member

Choose a reason for hiding this comment

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

I reported it here: numpy/numpy#30354


import numpy as np

from finchlite.finch_assembly.struct import MutableStructFType
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: Let's use one convention, with relative import as below.

np.generic,
"serialize_to_c",
"__attr__",
lambda fmt, obj: np.ctypeslib.as_ctypes(obj),
Copy link
Member

Choose a reason for hiding this comment

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

Can we have it as np.ctypeslib.as_ctypes(np.array(obj)), instead? It looks that 0-d arrays print your example correctly. Then we shouldn't need scalar_to_ctypes_copy.

@junikimm717 junikimm717 requested a review from mtsokol December 2, 2025 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants