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

feat: Add autoscale tail ignore via context menu [WIP] #147

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gselzer
Copy link
Collaborator

@gselzer gselzer commented Mar 1, 2025

This PR adds GUI functionality for autoscaling using ClimsPercentile, where the user can ignore the outlier samples of the dataset being displayed in autoscale:

Recording.2025-02-28.214155.mp4

I like the placement as a context menu because it means all autoscaling functionality appears within the same area of the canvas and because it normally does not take up any screen space.

Unfortunately, I do not know whether this design is possible with all our frontends, seeing as how I cannot figure out how to write a context menu in ipywidgets.

We might instead have to settle for adding this widget to the layouts added in #146...

@gselzer gselzer added the enhancement New feature or request label Mar 1, 2025
@gselzer gselzer requested a review from tlambert03 March 1, 2025 03:45
@gselzer gselzer self-assigned this Mar 1, 2025
Copy link

codecov bot commented Mar 1, 2025

Codecov Report

Attention: Patch coverage is 79.24528% with 11 lines in your changes missing coverage. Please review.

Project coverage is 80.68%. Comparing base (39f66b8) to head (a548b34).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/ndv/views/_qt/_array_view.py 79.24% 11 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #147   +/-   ##
=======================================
  Coverage   80.68%   80.68%           
=======================================
  Files          44       44           
  Lines        4209     4256   +47     
=======================================
+ Hits         3396     3434   +38     
- Misses        813      822    +9     

☔ 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.

@tlambert03
Copy link
Member

thanks @gselzer, this would indeed be great to have. Couple thoughts at this point:

  • I'm not sure a context menu is the correct way to show this here, it gives the strange appearance that the label is "actionable", and generally has the wrong look & feel. I think something like the QtPopup that I used to show an FPS selector back in v1 would be more appropriate (and generalizable: since I do need to get around to adding back those play buttons and will likely use that again).
    class QtPopup(QDialog):
    """A generic popup window."""
    def __init__(self, parent: QWidget | None = None) -> None:
    super().__init__(parent)
    self.setModal(False) # if False, then clicking anywhere else closes it
    self.setWindowFlags(Qt.WindowType.Popup | Qt.WindowType.FramelessWindowHint)
    self.frame = QFrame(self)
    layout = QVBoxLayout(self)
    layout.addWidget(self.frame)
    layout.setContentsMargins(0, 0, 0, 0)
    def show_above_mouse(self, *args: Any) -> None:
    """Show popup dialog above the mouse cursor position."""
    pos = QCursor().pos() # mouse position
    szhint = self.sizeHint()
    pos -= QPoint(szhint.width() // 2, szhint.height() + 14)
    self.move(pos)
    self.resize(self.sizeHint())
    self.show()
  • I would like to be able to set different percentiles for each end. (Does the mmstudio gui only have one number? I vaguely remember that being the case and not liking that... particularly with modern scmos cameras, you often want a different high number than low number)

@gselzer
Copy link
Collaborator Author

gselzer commented Mar 5, 2025

  • I think something like the QtPopup that I used to show an FPS selector back in v1 would be more appropriate (and generalizable: since I do need to get around to adding back those play buttons and will likely use that again).
  • I would like to be able to set different percentiles for each end.

Good ideas. Have updated in the latest commit:

image

If you're a fan of this now I'll investigate how/whether we can do similar things in wx (where I'm more confident) and jupyter (less confident)

@marktsuchida
Copy link

  • Does the mmstudio gui only have one number?

Yes, but I agree that separate settings are sometimes desirable.

I thought a bit about whether people would want the option of editing just one number, but then again, it doesn't make theoretical sense to use the same number for the read noise at the bottom and the shot noise at the top (for example). It would only be a heuristic convenience. Given that in many cases you'd only need a non-zero percentile for the upper limit (I think?), it seems fine (and simplest) to just have two fields.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants