search

pyapp-kit / superqt

Missing widgets and components for Qt-python
https://pyapp-kit.github.io/superqt/
BSD 3-Clause "New" or "Revised" License
210 stars 38 forks source link

When update range in `QLargeIntSpinBox` the value is not clipped to new range #220

Closed Czaki closed 1 year ago

Czaki commented 1 year ago

Describe the bug

When update range in QLargeIntSpinBox the value is not clipped to new range

https://github.com/napari/napari/pull/6479#issuecomment-1822181081

To Reproduce Steps to reproduce the behavior:

spin = QLargeIntSpinBox()
spin.setRange(0, 1000)
spin.setValue(500)
spin.setRange(0, 100)

and value remains 500 instead of 100

Expected behavior

Presented data should be clipped to the new range

cc @jni

tlambert03 commented 1 year ago

sounds like an easy fix. anyone want to take this one? if not, I should be able to get to it next week

Czaki commented 1 year ago

I'm not sure where I will have time. But if I got time to write this I will post a message here.

psobolewskiPhD commented 1 year ago

Would you just have setRange compare value to the range, and if outside it then call setValue? If that is on the general right track, then I can give it a whirl.

jni commented 1 year ago

I think that's the case @psobolewskiPhD. I wouldn't even compare, I'd just do, in setRange, setValue(clip(value, minval, maxval)) for a suitable definition of clip. ;)

Czaki commented 1 year ago

Would you just have setRange compare value to the range, and if outside it then call setValue? If that is on the general right track, then I can give it a whirl.

I suggest put this logic in setMinimum and setMaximum as this function are called in setRange

https://github.com/pyapp-kit/superqt/blob/b927159f49104a8cb90f205d4b82d7ed89a7e1cc/src/superqt/spinbox/_intspin.py#L66-L77

EDIT

ist looks like it could be nice to add check if minimum is lower than maximum in setRange

psobolewskiPhD commented 1 year ago

it could be nice to add check if minimum is lower than maximum in setRange

So this is the case where someone does setRange(100, 0) or similar. In this case is it better to raise a ValueError or just swap min and max so they are correct?

Czaki commented 1 year ago

So this is the case where someone does setRange(100, 0) or similar. In this case is it better to raise a ValueError or just swap min and max so they are correct?

I think that you should mimic Qt behaviour:

from qtpy.QtWidgets import QSpinBox

w = QSpinBox()

w.setRange(0, 100)

w.setRange(1000, 0)

w.minimum(), w.maximum()
Out[5]: (1000, 1000)