Skip to content

Conversation

@TimMonko
Copy link

@TimMonko TimMonko commented Jan 2, 2026

I noticed that in the camera popup widget in napari that the value of the label in slider labels no longer shows value outside the default range of the sliders (0,99). napari/napari#8534 (Should the decimal issue, which I proposed to fix in napari like this: https://github.com/napari/napari/pull/8535/files#diff-b7be858b0c42314c0f7fbaaa128a2e6a24822a61aa780d9374153c02a7a6b30f actually be fixed in superqt?)

I added a test here that all assertions fail on main, but pass with this PR.

I don't know if this code change is the correct fix because this is my first time looking inside superqt, but my thought process was that the label range needs to be connected to changing the slider range.

I'm only confused because this isn't how it used to work (see the screenshot in napari/napari#7626), but a quick scan of the file history isn't making anything stand out in this method as changed, so maybe this isn't the right place to fix this / the cause of the bug. It seems both the range and decimals somehow got disconnected from the label

@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.35%. Comparing base (9ce358e) to head (af7b948).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #320      +/-   ##
==========================================
- Coverage   86.40%   86.35%   -0.05%     
==========================================
  Files          49       49              
  Lines        3861     3862       +1     
==========================================
- Hits         3336     3335       -1     
- Misses        525      527       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Czaki
Copy link
Contributor

Czaki commented Jan 2, 2026

pyside2 fails looks unrelated

@tlambert03
Copy link
Member

Hi @TimMonko

If something has mysteriously changed at some point, and it's not clear where the fix should go, would you be able to run git bisect with superqt installed in editable mode and narrow down exactly what superqt commit caused the change? That might help us fill in the picture a bit and make sure we're fixing the right thing for you

@TimMonko
Copy link
Author

TimMonko commented Jan 2, 2026

@tlambert03 you have no idea how much you've indirectly taught me over the years. Today was my first time using git bisect ... and it was something I thought might be a good solution, but I was too scared to try it out. Wasn't too bad 😅

I found that 55b6639 was the first bad commit. It looks like there was a switch from using a spinbox to lineedit, which makes this more confusing to understand where the change should go than I expected.

So, I connected now to rangeChanged in SliderLabel. I think it should go there because its used by both QLabeledSlider and QLabeledRangeSlider? This does at least make sense why I noticed different behavior in a QLabeledRangeSlider widget (contrast limits in napari)

I'm not sure what to do about the test, maybe keep the one I have, or make a new direct test of SliderLabel?
Part of my confusion, I think, is that its not called QSliderLabel -- so I'm not sure if that has a name of intent for its meaning/use.

@Czaki
Copy link
Contributor

Czaki commented Jan 2, 2026

I was worried that it was that commit...

@tlambert03
Copy link
Member

Yeah... I was too. That commit has definitely cropped up for me in other places as well (stuff where it's maybe not a direct bug, but it has made things more complicated). One possible solution that I'd like to consider (in parallel perhaps) is to have two versions, or a feature flag on a single version... one that reverts the changes back to the spinBox, and one that uses the line edit to support scientific notation. It's somewhat unfortunate to add the complexity and side effects for a feature that I (personally) find to be a minority use case?

@Czaki
Copy link
Contributor

Czaki commented Jan 2, 2026

We need this in napari because QSpinBox do not support values above $2^{31}$, and there are images with such brghtness.
Also it do not work well with images with low brightness (ex. $10^{-5}$).
But except contrast limit I do not see place where we need custom line edit as label.

@tlambert03
Copy link
Member

tlambert03 commented Jan 2, 2026

We need this in napari because QSpinBox do not support values above 2^31, and there are images with such brghtness.

yes, @Czaki, if you read what I said, I wasn't proposing that we remove it altogether. but propose a way such that you can have what you need for your relatively edge use case, without imposing what has turned out to be a problematic design for the common use case. (i.e. have both)

@tlambert03
Copy link
Member

In any case @Czaki can you help @TimMonko here with this PR. Are you happy with this fix and test?

@tlambert03
Copy link
Member

btw, I did just remember what the other issue with #226 was (it's in ndv, with pyapp-kit/ndv#213 ... where we've actively pinned superqt to <0.7.5, because of that PR.) so @Czaki, if you are interested in having the scientific notation feature live in superqt (and not in napari), then it would be great if you could help out both with @TimMonko's issues here, and ideally help us figure out why that PR caused us to have segfaults over in ndv too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants