diff --git a/docs/release.md b/docs/release.md index 1f439a9c..d5dc568f 100644 --- a/docs/release.md +++ b/docs/release.md @@ -8,6 +8,10 @@ (unreleased)= ## Unreleased + +* Encode F-contiguous arrays in logical order for the vlen codecs and for delta, fixedscaleoffset, packbits, and categorize. They used to walk memory order, which did not match the logical layout. + By {user}`Corey Tasz `, {issue}`850`, {pr}`863` + --> (unreleased)= diff --git a/src/numcodecs/categorize.py b/src/numcodecs/categorize.py index c8199493..d6e4b580 100644 --- a/src/numcodecs/categorize.py +++ b/src/numcodecs/categorize.py @@ -53,8 +53,10 @@ def encode(self, buf): else: arr = ensure_ndarray(buf).view(self.dtype) - # flatten to simplify implementation - arr = arr.reshape(-1, order='A') + # flatten to simplify implementation, normalising to C order so the + # encoded stream holds elements in logical order regardless of the + # input's memory layout (see issue #850) + arr = np.ascontiguousarray(arr).reshape(-1) # setup output array enc = np.zeros_like(arr, dtype=self.astype) @@ -79,8 +81,9 @@ def decode(self, buf, out=None): for i, label in enumerate(self.labels): dec[enc == (i + 1)] = label - # handle output - return ndarray_copy(dec, out) + # handle output; the decoded stream holds elements in logical (C) + # order, so copy them into the destination in C order (see #850) + return ndarray_copy(dec, out, order='C') def get_config(self): return { diff --git a/src/numcodecs/compat.py b/src/numcodecs/compat.py index d1844e10..8b51d46d 100644 --- a/src/numcodecs/compat.py +++ b/src/numcodecs/compat.py @@ -174,8 +174,22 @@ def ensure_text(s, encoding="utf-8"): return s -def ndarray_copy(src, dst) -> NDArrayLike: - """Copy the contents of the array from `src` to `dst`.""" +def ndarray_copy(src, dst, order='A') -> NDArrayLike: + """Copy the contents of the array from `src` to `dst`. + + Parameters + ---------- + src : array-like + Source array, flattened before copying. + dst : array-like + Destination array. + order : {'A', 'C', 'F'}, optional + Memory layout used to interpret the flattened source data when + reshaping it to the destination's shape. The default 'A' preserves + the historical behavior of matching the destination's memory layout. + Pass 'C' when the source holds elements in logical (C) order, + regardless of the destination's memory layout (see issue #850). + """ if dst is None: # no-op @@ -194,10 +208,8 @@ def ndarray_copy(src, dst) -> NDArrayLike: # reshape source to match destination if src.shape != dst.shape: - if dst.flags.f_contiguous: - order = "F" - else: - order = "C" + if order == 'A': + order = 'F' if dst.flags.f_contiguous else 'C' src = src.reshape(dst.shape, order=order) # copy via numpy diff --git a/src/numcodecs/delta.py b/src/numcodecs/delta.py index e41f8be9..eae07930 100644 --- a/src/numcodecs/delta.py +++ b/src/numcodecs/delta.py @@ -53,8 +53,10 @@ def encode(self, buf): # normalise input arr = ensure_ndarray(buf).view(self.dtype) - # flatten to simplify implementation - arr = arr.reshape(-1, order='A') + # flatten to simplify implementation, normalising to C order so the + # encoded stream holds elements in logical order regardless of the + # input's memory layout (see issue #850) + arr = np.ascontiguousarray(arr).reshape(-1) # setup encoded output enc = np.empty_like(arr, dtype=self.astype) @@ -79,8 +81,9 @@ def decode(self, buf, out=None): # decode differences np.cumsum(enc, out=dec) - # handle output - return ndarray_copy(dec, out) + # handle output; the decoded stream holds elements in logical (C) + # order, so copy them into the destination in C order (see #850) + return ndarray_copy(dec, out, order='C') def get_config(self): # override to handle encoding dtypes diff --git a/src/numcodecs/fixedscaleoffset.py b/src/numcodecs/fixedscaleoffset.py index b1db5205..5ee3b8c6 100644 --- a/src/numcodecs/fixedscaleoffset.py +++ b/src/numcodecs/fixedscaleoffset.py @@ -84,8 +84,10 @@ def encode(self, buf): # normalise input arr = ensure_ndarray(buf).view(self.dtype) - # flatten to simplify implementation - arr = arr.reshape(-1, order='A') + # flatten to simplify implementation, normalising to C order so the + # encoded stream holds elements in logical order regardless of the + # input's memory layout (see issue #850) + arr = np.ascontiguousarray(arr).reshape(-1) # compute scale offset enc = (arr - self.offset) * self.scale @@ -109,8 +111,9 @@ def decode(self, buf, out=None): # convert dtype dec = dec.astype(self.dtype, copy=False) - # handle output - return ndarray_copy(dec, out) + # handle output; the decoded stream holds elements in logical (C) + # order, so copy them into the destination in C order (see #850) + return ndarray_copy(dec, out, order='C') def get_config(self): # override to handle encoding dtypes diff --git a/src/numcodecs/packbits.py b/src/numcodecs/packbits.py index bcdcf866..91dac845 100644 --- a/src/numcodecs/packbits.py +++ b/src/numcodecs/packbits.py @@ -33,8 +33,10 @@ def encode(self, buf): # normalise input arr = ensure_ndarray(buf).view(bool) - # flatten to simplify implementation - arr = arr.reshape(-1, order='A') + # flatten to simplify implementation, normalising to C order so the + # encoded stream holds elements in logical order regardless of the + # input's memory layout (see issue #850) + arr = np.ascontiguousarray(arr).reshape(-1) # determine size of packed data n = arr.size @@ -78,5 +80,6 @@ def decode(self, buf, out=None): # view as boolean array dec = dec.view(bool) - # handle destination - return ndarray_copy(dec, out) + # handle destination; the decoded stream holds elements in logical + # (C) order, so copy them into the destination in C order (see #850) + return ndarray_copy(dec, out, order='C') diff --git a/src/numcodecs/vlen.pyx b/src/numcodecs/vlen.pyx index f51e8cad..7f63d24f 100644 --- a/src/numcodecs/vlen.pyx +++ b/src/numcodecs/vlen.pyx @@ -40,8 +40,7 @@ def check_out_param(out, n_items): raise TypeError('out must be 1-dimensional array') if out.dtype != object: raise ValueError('out must be object array') - out = out.reshape(-1, order='A') - if out.shape[0] < n_items: + if out.size < n_items: raise ValueError('out is too small') return out @@ -87,8 +86,10 @@ class VLenUTF8(Codec): object o unicode u - # normalise input - input_values = np.asarray(buf, dtype=object).reshape(-1, order='A') + # normalise input to C order, so the encoded stream holds items + # in logical order regardless of the input's memory layout + # (see issue #850) + input_values = np.ascontiguousarray(buf, dtype=object).reshape(-1) # determine number of items n_items = input_values.shape[0] @@ -154,11 +155,11 @@ class VLenUTF8(Codec): # load number of items n_items = load_le32(data) - # setup output + # setup output; always decode into a fresh 1-D array, which holds + # the items in logical (stream) order if out is not None: out = check_out_param(out, n_items) - else: - out = np.empty(n_items, dtype=object) + dec = np.empty(n_items, dtype=object) # iterate and decode - N.B., do not try to cast `out` as object[:] # as this causes segfaults, possibly similar to @@ -171,10 +172,15 @@ class VLenUTF8(Codec): data += HEADER_LENGTH if data + L > data_end: raise ValueError('corrupt buffer, data seem truncated') - out[i] = PyUnicode_FromStringAndSize(data, L) + dec[i] = PyUnicode_FromStringAndSize(data, L) data += L - return out + if out is not None: + # place the decoded items into the caller's buffer in logical + # (C) order (see issue #850) + out.flat[:n_items] = dec + return out + return dec class VLenBytes(Codec): @@ -217,8 +223,10 @@ class VLenBytes(Codec): bytearray out char* data - # normalise input - values = np.asarray(buf, dtype=object).reshape(-1, order='A') + # normalise input to C order, so the encoded stream holds items + # in logical order regardless of the input's memory layout + # (see issue #850) + values = np.ascontiguousarray(buf, dtype=object).reshape(-1) # determine number of items n_items = values.shape[0] @@ -283,11 +291,11 @@ class VLenBytes(Codec): # load number of items n_items = load_le32(data) - # setup output + # setup output; always decode into a fresh 1-D array, which holds + # the items in logical (stream) order if out is not None: out = check_out_param(out, n_items) - else: - out = np.empty(n_items, dtype=object) + dec = np.empty(n_items, dtype=object) # iterate and decode - N.B., do not try to cast `out` as object[:] # as this causes segfaults, possibly similar to @@ -300,10 +308,15 @@ class VLenBytes(Codec): data += HEADER_LENGTH if data + L > data_end: raise ValueError('corrupt buffer, data seem truncated') - out[i] = PyBytes_FromStringAndSize(data, L) + dec[i] = PyBytes_FromStringAndSize(data, L) data += L - return out + if out is not None: + # place the decoded items into the caller's buffer in logical + # (C) order (see issue #850) + out.flat[:n_items] = dec + return out + return dec class VLenArray(Codec): @@ -361,8 +374,10 @@ class VLenArray(Codec): const Py_buffer* value_pb object o - # normalise input - values = np.asarray(buf, dtype=object).reshape(-1, order='A') + # normalise input to C order, so the encoded stream holds items + # in logical order regardless of the input's memory layout + # (see issue #850) + values = np.ascontiguousarray(buf, dtype=object).reshape(-1) # determine number of items n_items = values.shape[0] @@ -439,11 +454,11 @@ class VLenArray(Codec): # load number of items n_items = load_le32(data) - # setup output + # setup output; always decode into a fresh 1-D array, which holds + # the items in logical (stream) order if out is not None: out = check_out_param(out, n_items) - else: - out = np.empty(n_items, dtype=object) + dec = np.empty(n_items, dtype=object) # iterate and decode - N.B., do not try to cast `out` as object[:] # as this causes segfaults, possibly similar to @@ -463,7 +478,12 @@ class VLenArray(Codec): v_pb = PyMemoryView_GET_BUFFER(v_mv) memcpy(v_pb.buf, data, L) - out[i] = v + dec[i] = v data += L - return out + if out is not None: + # place the decoded items into the caller's buffer in logical + # (C) order (see issue #850) + out.flat[:n_items] = dec + return out + return dec diff --git a/tests/common.py b/tests/common.py index e3c9cfb3..b07ce967 100644 --- a/tests/common.py +++ b/tests/common.py @@ -28,15 +28,15 @@ ] -def compare_arrays(arr, res, precision=None): +def compare_arrays(arr, res, precision=None, order='A'): # ensure numpy array with matching dtype res = ensure_ndarray(res).view(arr.dtype) - # convert to correct shape - if arr.flags.f_contiguous: - order = 'F' - else: - order = 'C' + # convert to correct shape; 'A' keeps the historical behavior of matching + # the original array's memory layout, while 'C' expects the decoded data + # in logical (C) order (see issue #850) + if order == 'A': + order = 'F' if arr.flags.f_contiguous else 'C' res = res.reshape(arr.shape, order=order) # exact compare @@ -48,7 +48,7 @@ def compare_arrays(arr, res, precision=None): assert_array_almost_equal(arr, res, decimal=precision) -def check_encode_decode(arr, codec, precision=None): +def check_encode_decode(arr, codec, precision=None, order='A'): # N.B., watch out here with blosc compressor, if the itemsize of # the source buffer is different then the results of encoding # (i.e., compression) may be different. Hence we *do not* require that @@ -61,25 +61,25 @@ def check_encode_decode(arr, codec, precision=None): # test encoding of numpy array enc = codec.encode(arr) dec = codec.decode(enc) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test encoding of bytes - buf = arr.tobytes(order='A') + buf = arr.tobytes(order=order) enc = codec.encode(buf) dec = codec.decode(enc) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test encoding of bytearray - buf = bytearray(arr.tobytes(order='A')) + buf = bytearray(arr.tobytes(order=order)) enc = codec.encode(buf) dec = codec.decode(enc) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test encoding of array.array - buf = array.array('b', arr.tobytes(order='A')) + buf = array.array('b', arr.tobytes(order=order)) enc = codec.encode(buf) dec = codec.decode(enc) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # decoding should support any object exporting the buffer protocol, @@ -88,38 +88,38 @@ def check_encode_decode(arr, codec, precision=None): # test decoding of raw bytes dec = codec.decode(enc_bytes) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test decoding of bytearray dec = codec.decode(bytearray(enc_bytes)) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test decoding of array.array buf = array.array('b', enc_bytes) dec = codec.decode(buf) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test decoding of numpy array buf = np.frombuffer(enc_bytes, dtype='u1') dec = codec.decode(buf) - compare_arrays(arr, dec, precision=precision) + compare_arrays(arr, dec, precision=precision, order=order) # test decoding directly into numpy array out = np.empty_like(arr) codec.decode(enc_bytes, out=out) - compare_arrays(arr, out, precision=precision) + compare_arrays(arr, out, precision=precision, order=order) # test decoding directly into bytearray out = bytearray(arr.nbytes) codec.decode(enc_bytes, out=out) # noinspection PyTypeChecker - compare_arrays(arr, out, precision=precision) + compare_arrays(arr, out, precision=precision, order=order) -def assert_array_items_equal(res, arr): +def assert_array_items_equal(res, arr, order='A'): assert isinstance(res, np.ndarray) - res = res.reshape(-1, order='A') - arr = arr.reshape(-1, order='A') + res = res.reshape(-1, order=order) + arr = arr.reshape(-1, order=order) assert res.shape == arr.shape assert res.dtype == arr.dtype @@ -137,18 +137,18 @@ def assert_array_items_equal(res, arr): assert a == r -def check_encode_decode_array(arr, codec): +def check_encode_decode_array(arr, codec, order='A'): enc = codec.encode(arr) dec = codec.decode(enc) - assert_array_items_equal(arr, dec) + assert_array_items_equal(arr, dec, order=order) out = np.empty_like(arr) codec.decode(enc, out=out) - assert_array_items_equal(arr, out) + assert_array_items_equal(arr, out, order=order) enc = codec.encode(arr) dec = codec.decode(ensure_ndarray(enc)) - assert_array_items_equal(arr, dec) + assert_array_items_equal(arr, dec, order=order) def check_config(codec): diff --git a/tests/test_categorize.py b/tests/test_categorize.py index 97aca977..11262579 100644 --- a/tests/test_categorize.py +++ b/tests/test_categorize.py @@ -24,12 +24,12 @@ def test_encode_decode(): # unicode dtype for arr in arrays: codec = Categorize(labels, dtype=arr.dtype) - check_encode_decode(arr, codec) + check_encode_decode(arr, codec, order='C') # object dtype for arr in arrays_object: codec = Categorize(labels, dtype=arr.dtype) - check_encode_decode_array(arr, codec) + check_encode_decode_array(arr, codec, order='C') def test_encode(): @@ -85,3 +85,23 @@ def test_errors(): Categorize(labels=['foo', 'bar'], dtype='S6') with pytest.raises(TypeError): Categorize(labels=['foo', 'bar'], dtype='U6', astype=object) + + +def test_encode_f_contiguous_logical_order(): + # regression test for https://github.com/zarr-developers/numcodecs/issues/850 + # the encoded stream must hold elements in logical (C) order, so encoding + # an F-contiguous array produces the same bytes as its C-ordered equivalent + for dtype in 'U1', object: + arr_f = np.asfortranarray(np.array([['a', 'b'], ['a', 'a']], dtype=dtype)) + arr_c = np.ascontiguousarray(arr_f) + codec = Categorize(labels=['a', 'b'], dtype=dtype) + assert_array_equal(codec.encode(arr_f), codec.encode(arr_c)) + + # a consumer reshaping the decoded 1-D stream in C order gets the array back + dec = codec.decode(codec.encode(arr_f)).reshape(arr_f.shape) + assert_array_equal(dec, arr_f) + + # decoding into an F-ordered output buffer lands values in the right places + out = np.empty(arr_f.shape, dtype=dtype, order='F') + codec.decode(codec.encode(arr_f), out=out) + assert_array_equal(out, arr_f) diff --git a/tests/test_delta.py b/tests/test_delta.py index df48d6d0..458706c3 100644 --- a/tests/test_delta.py +++ b/tests/test_delta.py @@ -25,7 +25,7 @@ def test_encode_decode(): for arr in arrays: codec = Delta(dtype=arr.dtype) - check_encode_decode(arr, codec) + check_encode_decode(arr, codec, order='C') def test_encode(): @@ -59,3 +59,22 @@ def test_errors(): Delta(dtype=object) with pytest.raises(ValueError): Delta(dtype='i8', astype=object) + + +def test_encode_f_contiguous_logical_order(): + # regression test for https://github.com/zarr-developers/numcodecs/issues/850 + # the encoded stream must hold elements in logical (C) order, so encoding + # an F-contiguous array produces the same bytes as its C-ordered equivalent + arr_f = np.asfortranarray(np.arange(12, dtype='