Fix Filedialog to work across (mac)OS (#1393)

* Always use dir instead of directory

* Wrap `FileDialog` methods for OS-specific calls

* Clean-up os-specific wrapper to check for linux only

* Lint

* Fix test for non native `FileDialog`

sleap/gui/dialogs/filedialog.py

@@ -7,15 +7,46 @@
 """
 
 import os, re, sys
-from pathlib import Path
 
+from functools import wraps
+from pathlib import Path
+from typing import Callable
 from qtpy import QtWidgets
 
 
+def os_specific_method(func) -> Callable:
+    """Check if native dialog should be used and update kwargs based on OS.
+
+    Native Mac/Win file dialogs add file extension based on selected file type but
+    non-native dialog (used for Linux) does not do this by default.
+    """
+
+    @wraps(func)
+    def set_dialog_type(cls, *args, **kwargs):
+        is_linux = sys.platform.startswith("linux")
+        env_var_set = os.environ.get("USE_NON_NATIVE_FILE", False)
+        cls.is_non_native = is_linux or env_var_set
+
+        if cls.is_non_native:
+            kwargs["options"] = kwargs.get("options", 0)
+            kwargs["options"] |= QtWidgets.QFileDialog.DontUseNativeDialog
+
+        # Make sure we don't send empty options argument
+        if "options" in kwargs and not kwargs["options"]:
+            del kwargs["options"]
+
+        return func(cls, *args, **kwargs)
+
+    return set_dialog_type
+
+
 class FileDialog:
     """Substitute for QFileDialog; see class methods for details."""
 
+    is_non_native = False
+
     @classmethod
+    @os_specific_method
     def open(cls, *args, **kwargs):
         """
         Wrapper for `QFileDialog.getOpenFileName()`
@@ -24,10 +55,10 @@ def open(cls, *args, **kwargs):
 
         Passes along everything except empty "options" arg.
         """
-        cls._non_native_if_set(kwargs)
         return QtWidgets.QFileDialog.getOpenFileName(*args, **kwargs)
 
     @classmethod
+    @os_specific_method
     def openMultiple(cls, *args, **kwargs):
         """
         Wrapper for `QFileDialog.getOpenFileNames()`
@@ -36,22 +67,21 @@ def openMultiple(cls, *args, **kwargs):
 
         Passes along everything except empty "options" arg.
         """
-        cls._non_native_if_set(kwargs)
         return QtWidgets.QFileDialog.getOpenFileNames(*args, **kwargs)
 
     @classmethod
+    @os_specific_method
     def save(cls, *args, **kwargs):
         """Wrapper for `QFileDialog.getSaveFileName()`
 
         Uses non-native file dialog if USE_NON_NATIVE_FILE env var set.
 
         Passes along everything except empty "options" arg.
         """
-        is_non_native = cls._non_native_if_set(kwargs)
 
         # The non-native file dialog doesn't add file extensions from the
         # file-type menu in the dialog, so we need to do this ourselves.
-        if is_non_native and "filter" in kwargs and "dir" in kwargs:
+        if cls.is_non_native and "filter" in kwargs and "dir" in kwargs:
             filename = kwargs["dir"]
             filters = kwargs["filter"].split(";;")
             if filters:
@@ -61,7 +91,7 @@ def save(cls, *args, **kwargs):
         filename, filter = QtWidgets.QFileDialog.getSaveFileName(*args, **kwargs)
 
         # Make sure filename has appropriate file extension.
-        if is_non_native and filter:
+        if cls.is_non_native and filter:
             fn = Path(filename)
             # Get extension from filter as list of "*.ext"
             match = re.findall("\*(\.[a-zA-Z0-9]+)", filter)
@@ -77,6 +107,7 @@ def save(cls, *args, **kwargs):
         return filename, filter
 
     @classmethod
+    @os_specific_method
     def openDir(cls, *args, **kwargs):
         """Wrapper for `QFileDialog.getExistingDirectory()`
 
@@ -85,20 +116,3 @@ def openDir(cls, *args, **kwargs):
         Passes along everything except empty "options" arg.
         """
         return QtWidgets.QFileDialog.getExistingDirectory(*args, **kwargs)
-
-    @staticmethod
-    def _non_native_if_set(kwargs) -> bool:
-        is_non_native = False
-        is_linux = sys.platform.startswith("linux")
-        env_var_set = os.environ.get("USE_NON_NATIVE_FILE", False)
-
-        if is_linux or env_var_set:
-            is_non_native = True
-            kwargs["options"] = kwargs.get("options", 0)
-            kwargs["options"] |= QtWidgets.QFileDialog.DontUseNativeDialog
-
-        # Make sure we don't send empty options argument
-        if "options" in kwargs and not kwargs["options"]:
-            del kwargs["options"]
-
-        return is_non_native

sleap/gui/dialogs/formbuilder.py

@@ -579,7 +579,7 @@ def _make_file_button(
             def select_file(*args, x=field):
                 filter = item.get("filter", "Any File (*.*)")
                 filename, _ = FileDialog.open(
-                    None, directory=None, caption="Open File", filter=filter
+                    None, dir=None, caption="Open File", filter=filter
                 )
                 if len(filename):
                     x.setText(filename)
@@ -588,7 +588,7 @@ def select_file(*args, x=field):
         elif item["type"].split("_")[-1] == "dir":
             # Define function for button to trigger
             def select_file(*args, x=field):
-                filename = FileDialog.openDir(None, directory=None, caption="Open File")
+                filename = FileDialog.openDir(None, dir=None, caption="Open File")
                 if len(filename):
                     x.setText(filename)
                 self.valueChanged.emit()

sleap/gui/learning/dialog.py

@@ -722,9 +722,12 @@ def save(
     ):
         """Save scripts and configs to run pipeline."""
         if output_dir is None:
-            models_dir = os.path.join(os.path.dirname(self.labels_filename), "/models")
+            labels_fn = Path(self.labels_filename)
+            models_dir = Path(labels_fn.parent, "models")
             output_dir = FileDialog.openDir(
-                None, directory=models_dir, caption="Select directory to save scripts"
+                None,
+                dir=models_dir.as_posix(),
+                caption="Select directory to save scripts",
             )
 
             if not output_dir:

tests/gui/test_filedialog.py

@@ -3,26 +3,38 @@
 
 from qtpy import QtWidgets
 
-from sleap.gui.dialogs.filedialog import FileDialog
+from sleap.gui.dialogs.filedialog import os_specific_method, FileDialog
 
 
 def test_non_native_dialog():
-    save_env_non_native = os.environ.get("USE_NON_NATIVE_FILE", None)
+    @os_specific_method
+    def dummy_function(cls, *args, **kwargs):
+        """This function returns the `kwargs` modified by the wrapper.
 
-    os.environ["USE_NON_NATIVE_FILE"] = ""
+        Args:
+            cls: The `FileDialog` class.
 
+        Returns:
+            kwargs: Modified by the wrapper.
+        """
+        return kwargs
+
+    FileDialog.dummy_function = dummy_function
+    save_env_non_native = os.environ.get("USE_NON_NATIVE_FILE", None)
+    os.environ["USE_NON_NATIVE_FILE"] = ""
     d = dict()
-    FileDialog._non_native_if_set(d)
+
+    # Wrapper doesn't mutate `d` outside of scope, so need to return `modified_d`
+    modified_d = FileDialog.dummy_function(FileDialog, d)
     is_linux = sys.platform.startswith("linux")
     if is_linux:
-        assert d["options"] == QtWidgets.QFileDialog.DontUseNativeDialog
+        assert modified_d["options"] == QtWidgets.QFileDialog.DontUseNativeDialog
     else:
-        assert "options" not in d
+        assert "options" not in modified_d
 
     os.environ["USE_NON_NATIVE_FILE"] = "1"
-    d = dict()
-    FileDialog._non_native_if_set(d)
-    assert d["options"] == QtWidgets.QFileDialog.DontUseNativeDialog
+    modified_d = FileDialog.dummy_function(FileDialog, d)
+    assert modified_d["options"] == QtWidgets.QFileDialog.DontUseNativeDialog
 
     if save_env_non_native is not None:
         os.environ["USE_NON_NATIVE_FILE"] = save_env_non_native