[proof of concept] make labelled range sliders expandable by modifying label range

[alisterburt]

Hey Talley, I wanted a labelled QSlider where the label could be used to update the slider range like your labelled double slider. This PR is a minimal change that allows this to work for QLabeledSlider if the label range is manually updated

I've opened this PR so that we can discuss if there might be a better approach/worth adding some convenience for on the labelled slider class. If you think this is okay as-is then I'll add some tests before merge

from qtpy.QtCore import Qt
from qtpy.QtWidgets import QApplication, QVBoxLayout, QWidget

from superqt import QLabeledSlider

app = QApplication([])

ORIENTATION = Qt.Orientation.Horizontal

w = QWidget()
w.setLayout(QVBoxLayout())

qls = QLabeledSlider(ORIENTATION)
qls.valueChanged.connect(lambda e: print("qls valueChanged", e))
qls.setRange(0, 100)
qls.setValue(30)

# update the label range - comment this line to revert to default behaviour
qls._label.setRange(0, 1000)

w.layout().addWidget(qls)

w.show()
app.exec_()

behaviour without this change (value can't be set outside of slider range (0, 100)):

label-range-not-working.mp4

behaviour with this change (range can be extended (value can be set to any value in label range (0, 1000), slider updates):

label-range-working.mp4

[codecov]

Codecov Report

Patch coverage: 60.00% and project coverage change: -0.09 ⚠️

Comparison is base (402d237) 85.91% compared to head (8f83091) 85.82%.

❗ Current head 8f83091 differs from pull request most recent head 52d63ed. Consider uploading reports for the commit 52d63ed to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #172      +/-   ##
==========================================
- Coverage   85.91%   85.82%   -0.09%     
==========================================
  Files          32       32              
  Lines        2641     2646       +5     
==========================================
+ Hits         2269     2271       +2     
- Misses        372      375       +3     

Impacted Files	Coverage Δ
src/superqt/sliders/_labeled.py	83.85% <60.00%> (-0.30%)	⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tlambert03]

Seems very reasonable...
@Czaki you expressed concern over in the other thread... what's your hesitation?

[Czaki]

All Qt sliders, spinboxes, etc, do not allow for silent go out of range. This is really unexpected behavior. There is also no update of docs for that.

So if anyone uses this widget as input, then they could write the following code that will assume that emitted value is in a given range.

So at least, it should be mentioned in the documentation.

Or a new class like QLabeledSliderDynamic should be added.

[tlambert03]

You say silently, but couldn't we make sure that the proper signal is emitted?

Most sliders don't have labels next to them in Qt, but with a label there, it's very similar to programmatically calling .setRange right? (And that's fully supported)

[Czaki]

Yes. But setRange is called from code. The user from GUI cannot change range.

[tlambert03]

Perhaps we could add a flag to the init method to gate this feature? Like extend_range_to_value

(Then the code writer would know whether or not they need to pay attention to the rangeChanged signal)

[Czaki]

Perhaps we could add a flag to the init method to gate this feature? Like extend_range_to_value

Yes. This is one of a possible solution.

[tlambert03]

@alisterburt could you do that please? Let's make it false by default so that this feature is opt in. And then add a test to make sure that the rangeChanged (or whatever it is) signal is emitted when the feature is on and the label is changed, but not otherwise.

[alisterburt]

Hi both, happy to gate in whatever way is best but it’s worth noting that with this implementation the behaviour is already gated:- unless the label (spinbox) range, not the slider range, is explicitly changed in code the user cannot specify a value outside the slider range in the label- if the label (spinbox) range is extended programmatically, the user can specify a value within that larger range and the slider will updateI can add an extra parameter to the init if we still feel it’s necessary but the current behaviour feels safe to me… what do we think?Sent from mobile - apologies for brevityOn 21 Jun 2023, at 12:13, Talley Lambert ***@***.***> wrote: @alisterburt could you do that please? Let's make it false by default so that this feature is opt in. And then add a test to make sure that the rangeChanged (or whatever it is) signal is emitted when the feature is on and the label is changed, but not otherwise. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[tlambert03]

unless the label (spinbox) range, not the slider range, is explicitly changed in code the user cannot specify a value outside the slider range in the label-

ohhhh, right! Sorry, I missed the importance of that line in your example:

# update the label range - comment this line to revert to default behaviour
qls._label.setRange(0, 1000)

can add an extra parameter to the init if we still feel it’s necessary but the current behaviour feels safe to me… what do we think?

That said, I still think we need a somewhat public exposure of this. I don't want napari, for example, accessing ._label.setRange to "opt in" to this behavior... since _label may change in the future.

[alisterburt]

cool! I'll cook something up now :)

[alisterburt]

something is going wrong here

editingFinished is the signal used to propagate label value changes down to the slider so I manually emit it for the test. This has the unintended side effect of changing slider._label.minimum().

I tried to fix this by modifying

superqt/src/superqt/sliders/_labeled.py

Lines 593 to 596 in 52d63ed

	else:
	self.setMinimum(min(self._slider.minimum(), self.minimum()))
	self.setMaximum(max(self._slider.maximum(), self.maximum()))
	self._slider.rangeChanged.connect(self.setRange)

but the value for the label minimum has already been reset by the time we get here - I can't figure out where it has been reset...

[alisterburt]

I had a go but got a little stuck, there is a side effect when emitting the editingFinished event which somehow resets slider._label.minimum() and slider._label.maximum() - I thought I had it pinned down (see comment in code above) but unfortunately that's not the culprit and I can't see where else this is being modified...

Would appreciate some help if possible - spent an hour or so bashing my head on this to no avail, happy to provide more context if needed