-
Notifications
You must be signed in to change notification settings - Fork 49
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
Filedialog widget for magicgui #23
Filedialog widget for magicgui #23
Conversation
this is totally how this should be, great point! makes everything easier (and let's us use the native file dialogs). So we basically just need to make our own new widget that has a LineEdit and a button. I'll propose some changes that you can review. |
Codecov Report
@@ Coverage Diff @@
## master #23 +/- ##
==========================================
+ Coverage 95.49% 95.52% +0.03%
==========================================
Files 4 4
Lines 754 871 +117
==========================================
+ Hits 720 832 +112
- Misses 34 39 +5
Continue to review full report at Codecov.
|
FileDialog with button proposal
I've merged your suggestion, so this is ready for review now. It still needs a couple of tests added. I think we should stick with the class name |
I'm not sure how to properly test the |
yeah, that is a tricky one! i'm not immediately sure... might need to play around with that. but one immediate thought is that you might be able to use the QTimer pattern here, and then search |
this sort of approach will get you a handle on the dialog during a test: def handle_dialog():
dialog = next(
child
for child in filewidget.children()
if isinstance(child, QFileDialog)
)
print(dialog)
dialog.reject() #still hangs
QtCore.QTimer().singleShot(400, handle_dialog)
filewidget._on_choose_clicked() but I still can't close it. It might be necessary to move away from the pattern of using static methods as I did, and instead make a filedialog, use setFileMode, then call |
Oh that's really weird @tlambert03 - your suggestion ONLY hangs on Mac. Edited to add: I've just put a |
I'm very happy with this overall! I'd like to get it in soon. To summarize where we're at:
|
That was in reference to @jni's https://github.com/napari/magicgui/issues/20#issuecomment-648504543 where he suggested that "it might be better to go with r/w for modes (existing file/new file), and Sequence[Path] for existing files." In other words, if the user wants to allow for selection of multiple files, they should annotate the argument as a |
Co-authored-by: Talley Lambert <talley.lambert@gmail.com>
regarding using |
Co-authored-by: Talley Lambert <talley.lambert@gmail.com>
magicgui/_qt.py
Outdated
OPTIONAL_FILE = "getSaveFileName" | ||
EXISTING_DIRECTORY = "getExistingDirectory" | ||
# Aliases | ||
R = "getOpenFileNames" |
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.
R = "getOpenFileNames" | |
R = "getOpenFileName" |
let's make this an alias for a single file. and in a followup (or here if you want to) we can do something like this:
@magicgui
def filepicker(filename=Sequence[Path("~")]):
...
and then in type2widget
return functools.partial(MagicFileDialog, mode='EXISTING_FILES')
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.
whoops, sorry. I committed this and it broke the test. can you fix that? Then I think this is good to go
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.
Ok
That's my feeling too, glad it's not just me feeling lazy |
thanks! |
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/interactive-dialog-box-for-napari-io-plugin/48332/2 |
This is for discussion of https://github.com/napari/magicgui/issues/20 QFileDialog picker.
Useful to know:
type2widget
, we need to checkisinstance
instead. This takes care of platform specific things like WindowsPath, etc.Still up for discussion:
Here's a
File dialog GUI is embedded
Embedded GUI disappears completely once you select a file
Pairing file dialog with a version of the call button might be better, this launches the file picker when you need it and doesn't disappear