checks: Add explicit conversion to int in wxpyimgview

[nilason]

Fixes wxpyimgview failure to run with Python 3.10:

wxpyimgview image=map.bmp percent=50
Traceback (most recent call last):
  File "/dev/grass/dist.aarch64-apple-darwin21.6.0/etc/wxpyimgview_gui.py", line 93, in redraw
    self.draw()
  File "/dev/grass/dist.aarch64-apple-darwin21.6.0/etc/wxpyimgview_gui.py", line 88, in draw
    dc.DrawBitmap(BitmapFromImage(image), x0, y0, False)
TypeError: DC.DrawBitmap(): arguments did not match any overloaded call:
  overload 1: argument 2 has unexpected type 'float'
  overload 2: argument 2 has unexpected type 'float'

Using:
Python 3.10.7 (v3.10.7:6cc6b13308, Sep 5 2022, 14:02:52) [Clang 13.0.0 (clang-1300.0.29.30)]
wxPython 4.2.0 osx-cocoa (phoenix) wxWidgets 3.2.0

[tmszi]

It would be more appropriate to create a wrapper class for wx.PaintDC inside gui/wxpython/gui_core/wrap.py module.

class PaintDC(wx.PaintDC):
    """Wrapper around wx.PaintDC to have more control
    over the widget on different platforms/wxpython versions"""

    def __init__(self, *args, **kwargs):
        wx.PaintDC.__init__(self, *args, **kwargs)

    def DrawBitmap(self, **kwargs):
        x = kwargs.get("x")
        y = kwargs.get("y")
        kwargs["x"] = int(x) if x else x
        kwargs["y"] = int(y) if y else y
        super().DrawBitmap(**kwargs)

and then inside scripts/wxpyimgview/wxpyimgview_gui.py module import this wrapper class.

...
from gui_core.wrap import BitmapFromImage, PaintDC  # noqa: E402
...
dc = PaintDC(self)
...
dc.DrawBitmap(
    bitmap=BitmapFromImage(image),
    x=x0,
    y=y0,
    useMask=False,
)

[nilason]

It would be more appropriate to create a wrapper class for wx.PaintDC inside gui/wxpython/gui_core/wrap.py module.

I'm not quite sure why that would be. AFAICT, the problem isn't changed (wxPython) API, but changed enforcement of types. It should always had been int, no? If that is the case, I don't see the benefit overcomplicating the code.

The conversion could as well be done earlier on:

x0 = int((size.GetWidth() - app.i_width) / 2)
y0 = int((size.GetHeight() - app.i_height) / 2)

[tmszi]

It would be more appropriate to create a wrapper class for wx.PaintDC inside gui/wxpython/gui_core/wrap.py module.

I'm not quite sure why that would be. AFAICT, the problem isn't changed (wxPython) API, but changed enforcement of types. It should always had been int, no? If that is the case, I don't see the benefit overcomplicating the code.

It would be an advantage if wx.PaintDC is or will be used in several parts of the code, then it is better if the conversion to an integer takes place in one place (it is used wrapper class with overridden DrawBitmap method).

[nilason]

It would be more appropriate to create a wrapper class for wx.PaintDC inside gui/wxpython/gui_core/wrap.py module.

I'm not quite sure why that would be. AFAICT, the problem isn't changed (wxPython) API, but changed enforcement of types. It should always had been int, no? If that is the case, I don't see the benefit overcomplicating the code.

It would be an advantage if wx.PaintDC is or will be used in several parts of the code, then it is better if the conversion to an integer takes place in one place (it is used wrapper class with overridden DrawBitmap method).

But even so, they will have to be manually changed, all eventual cases of use anyway. The chain of inheritance will not work automatically, or am I missing something obvious... I'm still not convinced :-)

[tmszi]

But even so, they will have to be manually changed, all eventual cases of use anyway. The chain of inheritance will not work automatically, or am I missing something obvious... I'm still not convinced :-)

When you look at gui/wxpython/gui_core/wrap.py module, we use same wrapper class (and same conversion to int in the overridden method(s)) for e.g.

Slider class:

grass/gui/wxpython/gui_core/wrap.py

Lines 172 to 182 in ffb0ce6

	def __init__(self, *args, **kwargs):
	args = convertToInt(argsOrKwargs=args)
	kwargs = convertToInt(argsOrKwargs=kwargs)
	wx.Slider.__init__(self, *args, **kwargs)
	def SetRange(self, minValue, maxValue):
	wx.Slider.SetRange(self, int(minValue), int(maxValue))
	def SetValue(self, value):
	wx.Slider.SetValue(self, int(value))

SpinCtrl class

https://github.com/OSGeo/grass/blob/main/gui/wxpython/gui_core/wrap.py#L185

PseudoDC class

https://github.com/OSGeo/grass/blob/main/gui/wxpython/gui_core/wrap.py#L631

Rect class

https://github.com/OSGeo/grass/blob/main/gui/wxpython/gui_core/wrap.py#L690

[petrasovaa]

While I generally like the wrappers given the continued changes in wx (and Python), looking at this particular case, I agree with @nilason, doing the int conversion right there is a more pragmatic approach. There doesn't seem to be any other case in current code where we could take advantage of the wrapper (I may be wrong). There are DrawBitmap functions, but some of them are called on wx.MemoryDC, so we would have to wrap multiple DC classes (wx.PseudoDC is already wrapped). The integer conversion in the other existing wrapper classes made sense because the wrappers were already in place, so adding the integer conversion to the wrapper methods resulted in smaller changes in code.

There are still places in GUI, which could potentially crash in Python 3.10, not sure how to consistently test that.

[petrasovaa]

Maybe this way?

The conversion could as well be done earlier on:

x0 = int((size.GetWidth() - app.i_width) / 2)
y0 = int((size.GetHeight() - app.i_height) / 2)

x0 = (size.GetWidth() - app.i_width) // 2
y0 = (size.GetHeight() - app.i_height) // 2

[tmszi]

While I generally like the wrappers given the continued changes in wx (and Python), looking at this particular case, I agree with @nilason, doing the int conversion right there is a more pragmatic approach. There doesn't seem to be any other case in current code where we could take advantage of the wrapper (I may be wrong). There are DrawBitmap functions, but some of them are called on wx.MemoryDC, so we would have to wrap multiple DC classes (wx.PseudoDC is already wrapped). The integer conversion in the other existing wrapper classes made sense because the wrappers were already in place, so adding the integer conversion to the wrapper methods resulted in smaller changes in code.

Ok, You are right.