`ak.to_buffers` doesn't declare endianness

[agoose77]

Version of Awkward Array

main

Description and code to reproduce

1. We should return the endianness
2. Should we use a namedtuple for safer API for users?
3. We should expect additional parameters in unpickling in future, and add a version number for breaking changes

[jpivarski]

The from_buffers function,

awkward/src/awkward/operations/ak_from_buffers.py

Lines 13 to 43 in 8d7a2c3

	def from_buffers(
	form,
	length,
	container,
	buffer_key="{form_key}-{attribute}",
	*,
	backend: str = "cpu",
	highlevel=True,
	behavior=None,
	):
	"""
	Args:
	form (#ak.forms.Form or str/dict equivalent): The form of the Awkward
	Array to reconstitute from named buffers.
	length (int): Length of the array. (The output of this function is always
	single-partition.)
	container (Mapping, such as dict): The str \u2192 Python buffers that
	represent the decomposed Awkward Array. This `container` is only
	assumed to have a `__getitem__` method that accepts strings as keys.
	buffer_key (str or callable): Python format string containing
	`"{form_key}"` and/or `"{attribute}"` or a function that takes these
	as keyword arguments and returns a string to use as a key for a buffer
	in the `container`.
	backend (str): Library to use to generate values that are
	put into the new array. The default, cpu, makes NumPy
	arrays, which are in main memory (e.g. not GPU). If all the values in
	`container` have the same `backend` as this, they won't be copied.
	highlevel (bool): If True, return an #ak.Array; otherwise, return
	a low-level #ak.contents.Content subclass.
	behavior (None or dict): Custom #ak.behavior for the output array, if
	high-level.

can take a new argument, byteorder, which takes the same values as np.dtype.byteorder, namely these:

• "=": native (default)
• "<": little-endian (can be distinct from "=", though it's not easy to find hardware in which that's true anymore)
• ">": big-endian
• maybe "!" as a synonym for big-endian (Python's struct uses that, as well as "@" as a synonym for "=")

It's fine for this to be a single decision made for all buffers, because it's very rare to get a set of buffers in which some are little-endian and others are big-endian.

It also makes sense for this to be a from_buffers argument, rather than something in the Form, because the Form is a (relatively) more high-level description: it says whether a list is ListArray or ListOffsetArray and whether a numeric type is int32 or int64, but not the kind of hardware architecture or file format it came from, which endianness is more connected to. I think of the endianness-correction step as being part of the chain that includes decompression. (It wouldn't be appropriate for Forms to describe a chain of transformations, either.) But since you only know how to byte-swap after you know what integer width a buffer will be interpreted with (byte-swapping int32 is not the same as byte-swapping int64), and users might not know that on a per-buffer basis, it needs to be an argument to from_buffers.

Should we use a namedtuple for safer API for users?

I'm not sure what namedtuple idea you're considering. (The list of arguments isn't that long, and I think it's better to have a flat list even when it is very long. Typical users only want to set a few arguments to non-default values, so it's not cumbersome in the calling code.)

We should expect additional parameters in unpickling in future, and add a version number for breaking changes

Pickling/unpickling happens in two places, for ak.Array and ak.Record:

awkward/src/awkward/highlevel.py

Lines 1433 to 1454 in 8d7a2c3

	def __getstate__(self):
	packed = ak.operations.to_packed(self._layout, highlevel=False)
	form, length, container = ak.operations.to_buffers(
	packed, buffer_key="{form_key}-{attribute}", form_key="node{id}"
	)
	if self._behavior is ak.behavior:
	behavior = None
	else:
	behavior = self._behavior
	return form.to_dict(), length, container, behavior
	def __setstate__(self, state):
	form, length, container, behavior = state
	layout = ak.operations.from_buffers(
	form,
	length,
	container,
	highlevel=False,
	buffer_key="{form_key}-{attribute}",
	)
	self.layout = layout
	self.behavior = behavior

awkward/src/awkward/highlevel.py

Lines 2067 to 2089 in 8d7a2c3

	def __getstate__(self):
	packed = ak.operations.to_packed(self._layout, highlevel=False)
	form, length, container = ak.operations.to_buffers(
	packed.array, buffer_key="{form_key}-{attribute}", form_key="node{id}"
	)
	if self._behavior is ak.behavior:
	behavior = None
	else:
	behavior = self._behavior
	return form.to_dict(), length, container, behavior, packed.at
	def __setstate__(self, state):
	form, length, container, behavior, at = state
	layout = ak.operations.from_buffers(
	form,
	length,
	container,
	highlevel=False,
	buffer_key="{form_key}-{attribute}",
	)
	layout = ak.record.Record(layout, at)
	self.layout = layout
	self.behavior = behavior

Unpacking a fixed-size state tuple was a bad idea, since adding another tuple item will break forward-compatibility. We should at least use the *args syntax @henryiii suggested today or explicitly unpack each state[0], state[1], etc. A version number (for pickles) is also a good idea, and since we have to break forward-compatibility in the current implementation anyway, let's break it by putting the version number first. (Quick, before Awkward v2 gets many users!)

After that, we want to try to ensure forward and backward-compatibility forever, and the version number (int) might or might not help to write the logic that adapts internal format changes so that they appear to be unchanged to the outside world. (Maybe old code + new pickle has fewer features, but it should not be impossible to read the array in some sense, and the way we introduce internal format changes can ensure that.)

[agoose77]

I'm not sure what namedtuple idea you're considering. (The list of arguments isn't that long, and I think it's better to have a flat list even when it is very long. Typical users only want to set a few arguments to non-default values, so it's not cumbersome in the calling code.)

This is more about avoiding breakages for users in future, by defining the API for to_buffers to return an object with attributes. Such an object is backwards compatible because it can be accessed like a tuple for existing users, but new users are encouraged to use attribute access over unpacking such that future changes are less likely to cause problems. That said, in this case, I'd probably just break the API if we have to, otherwise compatibility cycle it.

[jpivarski]

Oh! The above doesn't imply a change in to_buffers. It doesn't have to declare the endianness of the buffers it returns (as a fourth tuple-item in its return value).

Here are a few ideas for to_buffers and endianness:

• No change at all. Since the containers dict that to_buffers returns contains NumPy arrays of the appropriate dtypes, a user who wants non-native endianness could post-process the dict without having to look up dtypes in the Form. The same is not true of the input to from_buffers, which might contain non-NumPy buffers.
• Add a byteorder argument to to_buffers that does the post-processing for the user. (Be careful not to change them in-place, since to_buffers returns buffers that are shared with the original array.) This argument would direct what to_buffers does, but to_buffers is not obliged to pass that information through into its return value any more than it has to explain how its buffer_key, form_key, and id_start were configured to produce the form_keys they made:

awkward/src/awkward/operations/ak_to_buffers.py

Lines 11 to 14 in 8d7a2c3

	buffer_key="{form_key}-{attribute}",
	form_key="node{id}",
	*,
	id_start=0,

I've been assuming that from_buffers's byteorder argument would be a keyword-only argument, not something that it expects to pick up from to_buffer's output.

This particular change doesn't imply any API-breaking changes (new keyword-only arguments are not API-breaking). Are you considering some other possible change in the future? The output of to_buffers could change into a namedtuple and I would not consider that to be API-breaking (tuple-unpacking and *args-passing to from_buffers would still work with a length-3 namedtuple). I think that could happen at any time, and I don't see it as being strongly motivated. It wouldn't be bad, either.

[agoose77]

Oh! The above doesn't imply a change in to_buffers.

Oh! In which case, let's not add a named tuple. If we wanted to move in that direction, I'd suggest a custom object / dataclass instead (i.e. drop the tuple-ness deliberately).

Here are a few ideas for to_buffers and endianness:

My priority here is conversion-on-reading. The idea is that users might save data on platform X with little endianness, and load it on platform Y with big endianness. We want that to be as painless as possible. Given that endianness is a property of the system, we don't need to return it; users just need to store sys.byteorder alongside the data. We can accept this as an argument to from_buffers, like you suggest.

[jpivarski]

We could take the opinion that the buffers created by to_buffers and read by from_buffers are assumed to be little-endian unless otherwise directed by the byteorder argument. The byteorder would have no "="/"@" option: it would be either "<" or ">"/"!"`.

This incurs no penalty on little-endian machines (same as now), but a copy penalty on big-endian machines. (Neither to_buffers nor from_buffers can change the byte order in place, but byte-swapping is trivial compared to copying.) But then again, nearly all architecture nowadays is either little-endian or can run in both modes.

To do this, the two functions would just need to determine which endianness the current hardware has (there might already be something in _util for this), and add a copy-and-flip if native endianness is different from the byteorder argument. The arrays inside the Awkward Array itself should always be native endian for whatever the architecture is.

The boolean logic is simple,

("<" if sys.byteorder == "little" else ">") == byteorder   # after "!" → ">"

but nearly impossible to test. It's very hard to find a big-endian machine! (We can just write it and trust that it will work.)

What do you think?

[agoose77]

I love the simplicity and robustness of this approach for the default case; that users just want things to work by default.

I would want to preserve the ability to avoid this penalty if users know what they're doing. It might be sufficient to make this the default option, but allow both to_buffers and from_buffers to accept a byteorder argument that overrides the assumption that the output/input is little-endian.