Merge pull request #43 from petebankhead/refactoring

Refactoring & UI improvements

.github/workflows/gradle.yml

@@ -16,15 +16,15 @@ jobs:
     steps:
     - name: Checkout
       uses: actions/checkout@v3
-    - name: Set up JDK 11
+    - name: Set up JDK 17
       uses: actions/setup-java@v3
       with:
-        java-version: '11'
+        java-version: '17'
         distribution: 'temurin'
     - name: Validate Gradle wrapper
-      uses: gradle/wrapper-validation-action@v1.1.0
+      uses: gradle/wrapper-validation-action@v1
     - name: Build with Gradle
-      uses: gradle/gradle-build-action@v2.7.1
+      uses: gradle/gradle-build-action@v2
       with:
         arguments: build
     - name: Upload artifacts

.github/workflows/release.yml

@@ -13,15 +13,15 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v3
-      - name: Set up JDK 11
+      - name: Set up JDK 17
         uses: actions/setup-java@v3
         with:
-          java-version: '11'
+          java-version: '17'
           distribution: 'temurin'
       - name: Validate Gradle wrapper
-        uses: gradle/wrapper-validation-action@v1.1.0
+        uses: gradle/wrapper-validation-action@v1
       - name: Build with Gradle
-        uses: gradle/gradle-build-action@v2.7.1
+        uses: gradle/gradle-build-action@v2
         with:
           arguments: build
       - name: Release

CHANGELOG.md

@@ -2,9 +2,11 @@
 
 ## v0.3.0
 
-* Add support for QuPath v0.5
-* Enable auto-updating (see https://github.com/qupath/qupath-extension-wsinfer/issues/40)
-* Add support for local models (see https://github.com/qupath/qupath-extension-wsinfer/issues/32)
+* Compatibility with QuPath v0.5 (now the minimum version required)
+* Enable auto-updating the extension (https://github.com/qupath/qupath-extension-wsinfer/issues/40)
+* Support for local models (https://github.com/qupath/qupath-extension-wsinfer/issues/32)
+* Show model information, where available
+* Code refactoring and UI improvements
 
 ## v0.2.1
 

build.gradle

@@ -37,8 +37,8 @@ dependencies {
     // Main QuPath user interface jar.
     // Automatically includes other QuPath jars as subdependencies.
     implementation "io.github.qupath:qupath-gui-fx:${qupathVersion}"
-    implementation 'io.github.qupath:qupath-fxtras:0.1.0'
-    implementation 'org.commonmark:commonmark:0.21.0'
+    implementation libs.qupath.fxtras
+    implementation libs.bundles.markdown
 
     // For logging - the version comes from QuPath's version catalog at
     // https://github.com/qupath/qupath/blob/main/gradle/libs.versions.toml

gradle/wrapper/gradle-wrapper.properties

@@ -1,5 +1,5 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-7.5.1-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.4-bin.zip
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists

gradlew

@@ -205,6 +205,12 @@ set -- \
         org.gradle.wrapper.GradleWrapperMain \
         "$@"
 
+# Stop when "xargs" is not available.
+if ! command -v xargs >/dev/null 2>&1
+then
+    die "xargs is not available"
+fi
+
 # Use "xargs" to parse quoted args.
 #
 # With -n1 it outputs one arg per line, with the quotes and backslashes removed.

gradlew.bat

@@ -14,7 +14,7 @@
 @rem limitations under the License.
 @rem
 
-@if "%DEBUG%" == "" @echo off
+@if "%DEBUG%"=="" @echo off
 @rem ##########################################################################
 @rem
 @rem  Gradle startup script for Windows
@@ -25,7 +25,7 @@
 if "%OS%"=="Windows_NT" setlocal
 
 set DIRNAME=%~dp0
-if "%DIRNAME%" == "" set DIRNAME=.
+if "%DIRNAME%"=="" set DIRNAME=.
 set APP_BASE_NAME=%~n0
 set APP_HOME=%DIRNAME%
 
@@ -40,7 +40,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome
 
 set JAVA_EXE=java.exe
 %JAVA_EXE% -version >NUL 2>&1
-if "%ERRORLEVEL%" == "0" goto execute
+if %ERRORLEVEL% equ 0 goto execute
 
 echo.
 echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
@@ -75,13 +75,15 @@ set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar
 
 :end
 @rem End local scope for the variables with windows NT shell
-if "%ERRORLEVEL%"=="0" goto mainEnd
+if %ERRORLEVEL% equ 0 goto mainEnd
 
 :fail
 rem Set variable GRADLE_EXIT_CONSOLE if you need the _script_ return code instead of
 rem the _cmd.exe /c_ return code!
-if  not "" == "%GRADLE_EXIT_CONSOLE%" exit 1
-exit /b 1
+set EXIT_CODE=%ERRORLEVEL%
+if %EXIT_CODE% equ 0 set EXIT_CODE=1
+if not ""=="%GRADLE_EXIT_CONSOLE%" exit %EXIT_CODE%
+exit /b %EXIT_CODE%
 
 :mainEnd
 if "%OS%"=="Windows_NT" endlocal

settings.gradle

@@ -1,6 +1,6 @@
 pluginManagement {
     plugins {
-        id 'org.bytedeco.gradle-javacpp-platform' version '1.5.8'
+        id 'org.bytedeco.gradle-javacpp-platform' version '1.5.9'
     }
 }
 

src/main/java/qupath/ext/wsinfer/WSInfer.java

@@ -307,7 +307,7 @@ private static Translator<Image, Classifications> buildTranslator(WSInferModel w
     private static Criteria<Image, Classifications> buildCriteria(WSInferModel wsiModel, Translator<Image, Classifications> translator, Device device) {
         return Criteria.builder()
                 .optApplication(Application.CV.IMAGE_CLASSIFICATION)
-                .optModelPath(wsiModel.getTSFile().toPath())
+                .optModelPath(wsiModel.getTorchScriptFile().toPath())
                 .optEngine("PyTorch")
                 .setTypes(Image.class, Classifications.class)
                 .optTranslator(translator)

src/main/java/qupath/ext/wsinfer/models/WSInferModel.java

@@ -46,10 +46,22 @@ public class WSInferModel {
     @SerializedName("hf_revision")
     String hfRevision;
 
+    /**
+     * Get a displayable name for the model.
+     * @return
+     */
     public String getName() {
         return hfRepoId;
     }
 
+    /**
+     * Get a description, if available (may be null).
+     * @return
+     */
+    public String getDescription() {
+        return description;
+    }
+
     /**
      * Get the configuration. Note that this may be null.
      * @return the model configuration, or null.
@@ -65,23 +77,35 @@ public WSInferModelConfiguration getConfiguration() {
      * Remove the cached model files.
      */
     public synchronized void removeCache() {
-        getTSFile().delete();
-        getCFFile().delete();
+        removeIfFound(getTorchScriptFile(), getConfigFile(), getReadMeFile());
+    }
+
+    private static void removeIfFound(File... files) {
+        for (var file : files) {
+            if (file != null && file.isFile()) {
+                try {
+                    logger.debug("Deleting file {}", file);
+                    file.delete();
+                } catch (Exception e) {
+                    logger.error("Unable to delete file {}", file, e);
+                }
+            }
+        }
     }
 
     /**
      * Get the torchscript file. Note that it is not guaranteed that the model has been downloaded.
      * @return path to torchscript pt file in cache dir
      */
-    public File getTSFile() {
+    public File getTorchScriptFile() {
         return getFile("torchscript_model.pt");
     }
 
     /**
      * Get the configuration file. Note that it is not guaranteed that the model has been downloaded.
      * @return path to model config file in cache dir
      */
-    public File getCFFile() {
+    public File getConfigFile() {
         return getFile("config.json");
     }
 
@@ -90,7 +114,7 @@ public File getCFFile() {
      * Get the configuration file. Note that it is not guaranteed that the model has been downloaded.
      * @return path to model config file in cache dir
      */
-    public File getREADMEFile() {
+    public File getReadMeFile() {
         return getFile("README.md");
     }
 
@@ -99,7 +123,7 @@ public File getREADMEFile() {
      * @return true if the files exist and the SHA matches, and the config is valid.
      */
     public boolean isValid() {
-        return getTSFile().exists() && checkModifiedTimes() && getConfiguration() != null;
+        return getTorchScriptFile().exists() && checkModifiedTimes() && getConfiguration() != null;
     }
 
     /**
@@ -110,7 +134,7 @@ public boolean isValid() {
      */
     private boolean checkModifiedTimes() {
         try {
-            return Files.getLastModifiedTime(getTSFile().toPath())
+            return Files.getLastModifiedTime(getTorchScriptFile().toPath())
                     .compareTo(Files.getLastModifiedTime(getPointerFile().toPath())) < 0;
         } catch (IOException e) {
             logger.error("Cannot get last modified time");
@@ -131,7 +155,7 @@ File getModelDirectory() {
     }
 
     private WSInferModelConfiguration tryToLoadConfiguration() {
-        var cfFile = getCFFile();
+        var cfFile = getConfigFile();
         if (cfFile.exists()) {
             try (var reader = Files.newBufferedReader(cfFile.toPath(), StandardCharsets.UTF_8)) {
                 return GsonTools.getInstance().fromJson(reader, WSInferModelConfiguration.class);
@@ -155,7 +179,7 @@ private static String checkSumSHA256(File file) throws IOException, NoSuchAlgori
      */
     private boolean checkSHAMatches() {
         try {
-            String shaDown = checkSumSHA256(getTSFile());
+            String shaDown = checkSumSHA256(getTorchScriptFile());
             // this is the format
             //        Result: version https://git-lfs.github.com/spec/v1
             //        oid sha256:fffeeecb4282b61b2b699c6dfcd8f76c30c8ca1af9800fa78f5d81fc0b78a4e2
@@ -167,7 +191,7 @@ private boolean checkSHAMatches() {
                 return false;
             }
         } catch (IOException | NoSuchAlgorithmException e) {
-            logger.error("Unable to generate SHA for {}", getTSFile(), e);
+            logger.error("Unable to generate SHA for {}", getTorchScriptFile(), e);
             return false;
         }
         return true;

src/main/java/qupath/ext/wsinfer/models/WSInferModelLocal.java

@@ -43,11 +43,11 @@ private WSInferModelLocal(File modelDirectory) throws IOException {
         this.modelDirectory = modelDirectory;
         this.hfRepoId = modelDirectory.getName();
         List<File> files = Arrays.asList(Objects.requireNonNull(modelDirectory.listFiles()));
-        if (!files.contains(getCFFile())) {
-            throw new IOException(resources.getString("error.localModel") + ": " + getCFFile().toString());
+        if (!files.contains(getConfigFile())) {
+            throw new IOException(resources.getString("error.localModel") + ": " + getConfigFile().toString());
         }
-        if (!files.contains(getTSFile())) {
-            throw new IOException(resources.getString("error.localModel") + ": " + getTSFile().toString());
+        if (!files.contains(getTorchScriptFile())) {
+            throw new IOException(resources.getString("error.localModel") + ": " + getTorchScriptFile().toString());
         }
     }
 
@@ -58,7 +58,7 @@ File getModelDirectory() {
 
     @Override
     public boolean isValid() {
-        return getTSFile().exists() && getConfiguration() != null;
+        return getTorchScriptFile().exists() && getConfiguration() != null;
     }
 
     @Override

src/main/java/qupath/ext/wsinfer/models/WSInferUtils.java

@@ -19,7 +19,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import qupath.ext.wsinfer.ui.WSInferPrefs;
-import qupath.fx.dialogs.Dialogs;
 import qupath.lib.io.GsonTools;
 
 import java.io.File;
@@ -83,9 +82,17 @@ private static void addLocalModels(WSInferModelCollection cachedModelCollection,
             if (model.isDirectory()) {
                 try {
                     var localModel = WSInferModelLocal.createInstance(model);
-                    cachedModelCollection.getModels().put(localModel.getName(), localModel);
+                    if (cachedModelCollection.getModels().put(localModel.getName(), localModel) != null) {
+                        logger.warn("Replaced model {} with local version", localModel.getName());
+                    } else {
+                        logger.info("Added local model {}", localModel.getName());
+                    }
                 } catch (IOException e) {
-                    Dialogs.showErrorNotification(resources.getString("title"), e);
+                    // This can occur if non-model directories are found in the current directory -
+                    // especially if the user is *typing* the directory, so there are intermediate
+                    // steps with directories that don't contain models; that's why we log rather than
+                    // show a dialog or notification.
+                    logger.warn(e.getMessage(), e);
                 }
             }
         }

src/main/java/qupath/ext/wsinfer/ui/WSInferCommand.java

@@ -23,7 +23,7 @@
 import javafx.stage.Stage;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import qupath.lib.gui.ExtensionClassLoader;
+import qupath.fx.utils.FXUtils;
 import qupath.lib.gui.QuPathGUI;
 import qupath.fx.dialogs.Dialogs;
 
@@ -54,14 +54,16 @@ public void run() {
         if (stage == null) {
             try {
                 stage = createStage();
+                stage.show();
+                FXUtils.retainWindowPosition(stage);
             } catch (IOException e) {
                 Dialogs.showErrorMessage(resources.getString("title"),
                         resources.getString("error.window"));
                 logger.error(e.getMessage(), e);
                 return;
             }
-        }
-        stage.show();
+        } else
+            stage.show();
     }
 
     private Stage createStage() throws IOException {
@@ -87,10 +89,18 @@ private Stage createStage() throws IOException {
         stage.setScene(scene);
         stage.setResizable(false);
 
-        root.heightProperty().addListener((v, o, n) -> stage.sizeToScene());
+        root.heightProperty().addListener((v, o, n) -> handleStageHeightChange());
 
         return stage;
     }
 
+    private void handleStageHeightChange() {
+        stage.sizeToScene();
+        // This fixes a bug where the stage would migrate to the corner of a screen if it is
+        // resized, hidden, then shown again
+        if (stage.isShowing() && Double.isFinite(stage.getX()) && Double.isFinite(stage.getY()))
+            FXUtils.retainWindowPosition(stage);
+    }
+
 
 }