Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn off adaptive step if "step" option is given #425

Merged
merged 3 commits into from
Jun 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion magicgui/backends/_qtpy/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,10 @@ def _mgui_set_step(self, value: float):
self._readout_widget.setSingleStep(value)

def _mgui_get_adaptive_step(self) -> bool:
return self._readout_widget.stepType() == QtW.QAbstractSpinBox.AdaptiveStep
return (
self._readout_widget.stepType()
== QtW.QAbstractSpinBox.AdaptiveDecimalStepType
)

def _mgui_set_adaptive_step(self, value: bool):
self._readout_widget.setStepType(
Expand Down
34 changes: 23 additions & 11 deletions magicgui/widgets/_bases/ranged_widget.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class RangedWidget(ValueWidget):
The maximum allowable value, by default 999 (or `value` if `value` is greater
than 999)
step : float, optional
The step size for incrementing the value, by default 1
The step size for incrementing the value, by default adaptive step is used
"""

_widget: _protocols.RangedWidgetProtocol
Expand All @@ -29,8 +29,7 @@ def __init__(
self,
min: Union[float, _Unset] = UNSET,
max: Union[float, _Unset] = UNSET,
step: float = 1,
adaptive_step: bool = True,
step: Union[float, _Unset, None] = UNSET,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you motivate me to why you add None and _Unset here? One of them should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because min and max are also initialized with UNSET... I guess UNSET is for value only (because it might be set to None intentionally) so should we just change the defaults of min, max and step to None?

**kwargs,
): # sourcery skip: avoid-builtin-shadow
for key in ("maximum", "minimum"):
Expand All @@ -50,8 +49,12 @@ def __init__(

tmp_val = float(val if val not in (UNSET, None) else 1)

self.step = step
self.adaptive_step = adaptive_step
if step is UNSET or step is None:
self.step = None
self._widget._mgui_set_step(1)
else:
self.step = cast(float, step)

self.min: float = (
cast(float, min) if min is not UNSET else builtins.min(0, tmp_val)
)
Expand Down Expand Up @@ -99,22 +102,31 @@ def max(self, value: float):
self._widget._mgui_set_max(value)

@property
def step(self) -> float:
"""Step size for widget values."""
def step(self) -> Union[float, None]:
"""Step size for widget values (None if adaptive step is turned on)."""
if self._widget._mgui_get_adaptive_step():
return None
return self._widget._mgui_get_step()

@step.setter
def step(self, value: float):
self._widget._mgui_set_step(value)
def step(self, value: Union[float, None]):
if value is None:
self._widget._mgui_set_adaptive_step(True)
else:
self._widget._mgui_set_adaptive_step(False)
self._widget._mgui_set_step(value)

@property
def adaptive_step(self):
"""Whether the step size is adaptive."""
return self._widget._mgui_get_adaptive_step()
return self.step is None

@adaptive_step.setter
def adaptive_step(self, value: bool):
self._widget._mgui_set_adaptive_step(value)
if value:
self.step = None
else:
self.step = self._widget._mgui_get_step()

@property
def range(self) -> Tuple[float, float]:
Expand Down
23 changes: 19 additions & 4 deletions tests/test_widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ def test_basic_widget_attributes():
assert widget.options == {
"max": 999,
"min": 0,
"step": 1,
"step": None,
"enabled": False,
"visible": False,
}
Expand Down Expand Up @@ -581,15 +581,30 @@ def test_range_negative_value():


def test_adaptive():
"""Turn on and off adaptive step."""

rw = widgets.SpinBox()
assert rw.adaptive_step
assert rw.step is None
rw.adaptive_step = False
assert not rw.adaptive_step
assert rw.step == 1
rw.step = None
assert rw.adaptive_step
assert rw.step is None
rw.step = 3
assert not rw.adaptive_step
assert rw.step == 3


def test_adaptive2():
rw = widgets.SpinBox(adaptive_step=False)
rw = widgets.SpinBox(step=2)
assert not rw.adaptive_step
assert rw.step == 2
rw.adaptive_step = True
assert rw.adaptive_step
assert rw.step is None
rw.adaptive_step = False
assert not rw.adaptive_step
assert rw.step == 2


def test_exception_range_out_of_range():
Expand Down