xdf-modules / pyxdf

Python package for working with XDF files
BSD 2-Clause "Simplified" License
34 stars 16 forks source link

Fix reading numeric array data on big-endian hosts #98

Closed musicinmybrain closed 1 month ago

musicinmybrain commented 8 months ago

Fixes a test failure when the Fedora Linux package is built and tested on s390x, which is Fedora’s sole big-endian primary architecture:

=================================== FAILURES ===================================
__________________ test_load_file[example-files/minimal.xdf] ___________________
file = 'example-files/minimal.xdf'
    @pytest.mark.parametrize("file", files)
    def test_load_file(file):
        streams, header = load_xdf(file)

        if file.endswith("minimal.xdf"):
            assert header["info"]["version"][0] == "1.0"

            assert len(streams) == 2
            assert streams[0]["info"]["name"][0] == "SendDataC"
            assert streams[0]["info"]["type"][0] == "EEG"
            assert streams[0]["info"]["channel_count"][0] == "3"
            assert streams[0]["info"]["nominal_srate"][0] == "10"
            assert streams[0]["info"]["channel_format"][0] == "int16"
            assert streams[0]["info"]["stream_id"] == 0

            s = np.array([[192, 255, 238],
                          [12, 22, 32],
                          [13, 23, 33],
                          [14, 24, 34],
                          [15, 25, 35],
                          [12, 22, 32],
                          [13, 23, 33],
                          [14, 24, 34],
                          [15, 25, 35]], dtype=np.int16)
            t = np.array([5., 5.1, 5.2, 5.3, 5.4, 5.5, 5.6, 5.7, 5.8])
>           np.testing.assert_array_equal(streams[0]["time_series"], s)
pyxdf/test/test_data.py:42: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
args = (<built-in function eq>, array([[-16384,   -256,  -4608],
       [  3072,   5632,   8192],
       [  3328,   5888,   8...5,  35],
       [ 12,  22,  32],
       [ 13,  23,  33],
       [ 14,  24,  34],
       [ 15,  25,  35]], dtype=int16))
kwds = {'err_msg': '', 'header': 'Arrays are not equal', 'strict': False, 'verbose': True}
    @wraps(func)
    def inner(*args, **kwds):
        with self._recreate_cm():
>           return func(*args, **kwds)
E           AssertionError: 
E           Arrays are not equal
E           
E           Mismatched elements: 27 / 27 (100%)
E           Max absolute difference: 16576
E           Max relative difference: 255.
E            x: array([[-16384,   -256,  -4608],
E                  [  3072,   5632,   8192],
E                  [  3328,   5888,   8448],...
E            y: array([[192, 255, 238],
E                  [ 12,  22,  32],
E                  [ 13,  23,  33],...
/usr/lib64/python3.11/contextlib.py:81: AssertionError
=========================== short test summary info ============================
FAILED pyxdf/test/test_data.py::test_load_file[example-files/minimal.xdf] - A...
========================= 1 failed, 4 passed in 0.33s ==========================

Fortunately, it looks like this is the only place where the implementation accidentally assumes the host is little-endian.

Reference: https://numpy.org/doc/stable/reference/generated/numpy.frombuffer.html

musicinmybrain commented 2 months ago

This PR remains valid and necessary with 1.16.7.

cbrnr commented 2 months ago

@musicinmybrain sorry, I completely missed your PR! But I can cut a new release tomorrow, could you please add a change log entry?

musicinmybrain commented 2 months ago

@musicinmybrain sorry, I completely missed your PR! But I can cut a new release tomorrow, could you please add a change log entry?

Sure! Done. Thanks for looking at this PR.

musicinmybrain commented 2 months ago

I also reformatted the diff with ruff, which should satisfy the style check.

cbrnr commented 1 month ago

Thanks @musicinmybrain 🚀!