-
-
Notifications
You must be signed in to change notification settings - Fork 410
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
Move help actions under _qt
and remove lambdas
#6883
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6883 +/- ##
==========================================
- Coverage 92.45% 92.44% -0.02%
==========================================
Files 617 616 -1
Lines 55156 55150 -6
==========================================
- Hits 50993 50981 -12
- Misses 4163 4169 +6 ☔ View full report in Codecov by Sentry. |
cc @Czaki @DragaDoncila 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me! Thanks @lucyleeow ❤️
I apologise if this is me being a goldfish 🐠, but why are they moved to Qt? I see nothing Qt related in them, and presumably if we have a different front-end one day, these actions can be used unchanged? |
Nope, not me being a goldfish, just me not clicking through, or rather clicking "open in new tab" then not looking. 🤦
|
Anyway I'm not super sure I'm convinced about the design but it's not a hard thing to revert. |
The new layer menu actions have opened up some questions too. I'm just about to open a PR to start a discussion about this stuff. Please share your views there @jni (i'll ping you)!! |
Yeah they're not qt related but they are only useful with a GUI. Since we don't have a "non qt but defs gui" folder, we've decided to move these here so that they're explicitly considered if a time comes when we want to make them available via a different front end. I'm not in love with this either, but as Lucy says we've run into this question a few times so I think best is to merge this, and think about the separation of headless-gui-qt a bit more carefully in a broader sense. |
@jni I will add ready-to-merge here, but please feel free to request changes if you want more discussion |
# Description As menu actions are all one file (see napari#6848 and napari#6883), update the file docstrings to remove mention of 'non-qt' actions.
References and relevant issues
Closes #6744
Related: #6848 (comment)
Description
Move all help actions to
_qt/
and remove lambdas and replace with partials.