superqt icon indicating copy to clipboard operation
superqt copied to clipboard

test_cmap_draw_result fails on big endian (s390x)

Open penguinpee opened this issue 11 months ago • 7 comments

Describe the bug I first observed test_cmap_draw_result failing on big endian in version 0.7.2. When I updated the Fedora package to 0.7.3 I happened to hit a big endian builder on which that same test failed again.

=================================== FAILURES ===================================
____________________________ test_cmap_draw_result _____________________________
    def test_cmap_draw_result():
        """Test that the image drawn actually looks correct."""
        # draw into any QPaintDevice
        w = 100
        h = 20
        pix = QPixmap(w, h)
        cmap = Colormap("viridis")
        draw_colormap(pix, cmap)
    
        ary1 = cmap(np.tile(np.linspace(0, 1, w), (h, 1)), bytes=True)
        ary2 = qimage_to_array(pix.toImage())
    
        # there are some subtle differences between how qimage draws and how
        # cmap draws, so we can't assert that the arrays are exactly equal.
        # they are visually indistinguishable, and numbers are close within 4 (/255) values
        # and linux, for some reason, is a bit more different``
        atol = 8 if platform.system() == "Linux" else 4
>       np.testing.assert_allclose(ary1, ary2, atol=atol)
tests/test_cmap.py:64: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
args = (<function assert_allclose.<locals>.compare at 0x3ff5bfe4b80>, array([[[ 68,   1,  84, 255],
        [ 68,   3,  87, 2...
        ...,
        [231, 247, 255,  24],
        [231, 255, 255,  33],
        [231, 255, 255,  33]]], dtype=uint8))
kwds = {'equal_nan': True, 'err_msg': '', 'header': 'Not equal to tolerance rtol=1e-07, atol=8', 'verbose': True}
    @wraps(func)
    def inner(*args, **kwds):
        with self._recreate_cm():
>           return func(*args, **kwds)
E           AssertionError: 
E           Not equal to tolerance rtol=1e-07, atol=8
E           
E           Mismatched elements: 7720 / 8000 (96.5%)
E           Max absolute difference: 128
E           Max relative difference: 16.
E            x: array([[[ 68,   1,  84, 255],
E                   [ 68,   3,  87, 255],
E                   [ 69,   8,  91, 255],...
E            y: array([[[  0,  66, 255,  82],
E                   [  4,  66, 255,  90],
E                   [  8,  66, 255,  90],...
/usr/lib64/python3.13/contextlib.py:85: AssertionError

To Reproduce Steps to reproduce the behavior:

  1. Run tests on big endian arch (e.g. s390x)

Expected behavior Tests should have the same outcome independent of arch.

Desktop (please complete the following information):

  • Fedora Linux (all current releases; F40+)
  • PyQt6 6.9.0 and 6.8.1
  • Python 3.13.2

penguinpee avatar Apr 05 '25 09:04 penguinpee

Since access to big endian machines might be difficult to obtain, I can help out by running tests on s390x or collecting more information.

The full log of the build will be available for a limited time. But I can always fire off another scratch build, test patches, etc.

penguinpee avatar Apr 05 '25 09:04 penguinpee

Could you do visual testing? Just check if the problem is with the test or colormap are wrongly painted?

Czaki avatar Apr 05 '25 10:04 Czaki

I will try...

penguinpee avatar Apr 05 '25 12:04 penguinpee

I saved the images as PNG and they look identical. I also looked at the arrays the test compares. While ary1 is identical on both x86_64 and s390x, ary2, returned by qimage_to_array(pix.toImage()), is different.

On x86_64:

[[[ 66   0  82 255]
  [ 66   4  90 255]
  [ 66   8  90 255]
  ...
  [247 231  24 255]
  [255 231  33 255]
  [255 231  33 255]]

 [[ 66   0  82 255]
  [ 66   4  90 255]
  [ 66   8  90 255]
  ...
  [247 231  24 255]
  [255 231  33 255]
  [255 231  33 255]]

 [[ 66   0  82 255]
  [ 66   4  90 255]
  [ 66   8  90 255]
  ...
  [247 231  24 255]
  [255 231  33 255]
  [255 231  33 255]]

 ...

 [[ 66   0  82 255]
  [ 66   4  90 255]
  [ 66   8  90 255]
  ...
  [247 231  24 255]
  [255 231  33 255]
  [255 231  33 255]]

 [[ 66   0  82 255]
  [ 66   4  90 255]
  [ 66   8  90 255]
  ...
  [247 231  24 255]
  [255 231  33 255]
  [255 231  33 255]]

 [[ 66   0  82 255]
  [ 66   4  90 255]
  [ 66   8  90 255]
  ...
  [247 231  24 255]
  [255 231  33 255]
  [255 231  33 255]]]

On s390x:

[[[  0  66 255  82]
  [  4  66 255  90]
  [  8  66 255  90]
  ...
  [231 247 255  24]
  [231 255 255  33]
  [231 255 255  33]]

 [[  0  66 255  82]
  [  4  66 255  90]
  [  8  66 255  90]
  ...
  [231 247 255  24]
  [231 255 255  33]
  [231 255 255  33]]

 [[  0  66 255  82]
  [  4  66 255  90]
  [  8  66 255  90]
  ...
  [231 247 255  24]
  [231 255 255  33]
  [231 255 255  33]]

 ...

 [[  0  66 255  82]
  [  4  66 255  90]
  [  8  66 255  90]
  ...
  [231 247 255  24]
  [231 255 255  33]
  [231 255 255  33]]

 [[  0  66 255  82]
  [  4  66 255  90]
  [  8  66 255  90]
  ...
  [231 247 255  24]
  [231 255 255  33]
  [231 255 255  33]]

 [[  0  66 255  82]
  [  4  66 255  90]
  [  8  66 255  90]
  ...
  [231 247 255  24]
  [231 255 255  33]
  [231 255 255  33]]]

The columns appear to be ordered differently on s390x.

penguinpee avatar Apr 06 '25 11:04 penguinpee

--- a/src/superqt/utils/_img_utils.py
+++ b/src/superqt/utils/_img_utils.py
@@ -1,3 +1,5 @@
+import sys
+
 from typing import TYPE_CHECKING
 
 from qtpy.QtGui import QImage
@@ -37,4 +39,8 @@ def qimage_to_array(img: QImage) -> "np.ndarray":
     arr = np.frombuffer(b, np.uint8).reshape(h, w, c)
 
     # reverse channel colors for numpy
-    return arr.take([2, 1, 0, 3], axis=2)
+    # On big endian we need to specify a different order
+    if sys.byteorder == 'big':
+        return arr.take([1, 2, 3, 0], axis=2)
+    else:
+        return arr.take([2, 1, 0, 3], axis=2)

This solves it for me. Though, I'm not sure if that would just be masking an underlying issue.

penguinpee avatar Apr 06 '25 14:04 penguinpee

thanks @penguinpee! i would be happy to include that change. @Czaki, what do you think?

tlambert03 avatar Apr 06 '25 15:04 tlambert03

I think that it is a good idea. But in the branch for big endian I will put, a comment with reference to this Issue to clarify the reason, as we cannot test it on CI.

Czaki avatar Apr 06 '25 18:04 Czaki