From ba2ba6d06819bc3aa750d0ce83a96f3e9291d722 Mon Sep 17 00:00:00 2001
From: Remi Gau <remi_gau@hotmail.com>
Date: Sat, 13 Jul 2024 18:46:26 +0200
Subject: [PATCH] [FIX] fix octave create roi (#130)

* test with octave

* change name

* fix

* tweak

* fix several tests

* reset submod

* refactor set up

* update tests

* fix for matlab

* reuse set up
---
 .github/workflows/run_tests_octave.yml | 78 ++++++++++++++++++++++++++
 .github/workflows/tests_octave.m       | 22 ++++++++
 src/atlas/copyAtlasToSpmDir.m          |  5 --
 src/atlas/extractRoiFromAtlas.m        | 13 ++++-
 src/atlas/unzipAtlas.m                 | 12 +++-
 src/roi/thresholdToMask.m              |  6 +-
 tests/test_createRoi.m                 | 37 ++++--------
 tests/test_getPeakCoordinates.m        | 21 +------
 tests/test_isBinaryMask.m              | 42 ++------------
 tests/test_keepHemisphere.m            |  8 +--
 tests/test_labelClusters.m             | 21 ++-----
 tests/test_thresholdToMask.m           | 35 ++----------
 tests/utils/demoDir.m                  |  9 +++
 tests/utils/rmRetinoAtlas.m            |  2 +-
 tests/utils/setUpDemoData.m            | 16 ++++++
 tests/utils/tempDir.m                  |  6 ++
 16 files changed, 186 insertions(+), 147 deletions(-)
 create mode 100644 .github/workflows/run_tests_octave.yml
 create mode 100644 .github/workflows/tests_octave.m
 create mode 100644 tests/utils/demoDir.m
 create mode 100644 tests/utils/setUpDemoData.m
 create mode 100644 tests/utils/tempDir.m

diff --git a/.github/workflows/run_tests_octave.yml b/.github/workflows/run_tests_octave.yml
new file mode 100644
index 0000000..7037e5e
--- /dev/null
+++ b/.github/workflows/run_tests_octave.yml
@@ -0,0 +1,78 @@
+---
+name: tests and coverage with octave
+
+env:
+  OCTFLAGS: --no-gui --no-window-system --silent
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+on:
+  push:
+    branches:
+    - main
+  pull_request:
+    branches: ['*']
+  schedule:
+  - cron: 0 0 1 * *
+
+  # Allows you to run this workflow manually from the Actions tab
+  workflow_dispatch:
+
+jobs:
+  tests_octave:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Install CPP_ROI
+      uses: actions/checkout@v4
+      with:
+        submodules: true
+        fetch-depth: 0
+
+    - name: Install SPM
+      run: |
+        git clone https://github.com/spm/spm12.git --depth 1
+
+    - name: Install Moxunit and MOcov
+      run: |
+        git clone https://github.com/MOxUnit/MOxUnit.git --depth 1
+        git clone https://github.com/MOcov/MOcov.git --depth 1
+
+    - name: Install octave
+      run: |
+        sudo apt-get -y -qq update
+        sudo apt-get -y install \
+          octave \
+          liboctave-dev\
+          octave-common \
+          octave-io \
+          octave-image \
+          octave-signal \
+          octave-statistics
+        make -C MOxUnit install
+        make -C MOcov install
+
+    - name: Compile SPM
+      run: |
+        make -C spm12/src PLATFORM=octave distclean
+        make -C spm12/src PLATFORM=octave
+        make -C spm12/src PLATFORM=octave install
+        octave $OCTFLAGS --eval "addpath(fullfile(pwd, 'spm12')); savepath();"
+
+    - name: Add bids-matlab
+      run: make install_dev
+
+    - name: Run tests
+      run: |
+        octave $OCTFLAGS --eval "addpath(fullfile(pwd, 'tests', 'utils')); savepath();"
+        octave $OCTFLAGS --eval "cd(fullfile(getenv('GITHUB_WORKSPACE'), '.github', 'workflows')); run tests_octave;"
+
+    - name: Code coverage
+      uses: codecov/codecov-action@v4
+      with:
+        file: coverage.xml
+        flags: octave
+        name: codecov-umbrella
+        fail_ci_if_error: false
diff --git a/.github/workflows/tests_octave.m b/.github/workflows/tests_octave.m
new file mode 100644
index 0000000..2d2af52
--- /dev/null
+++ b/.github/workflows/tests_octave.m
@@ -0,0 +1,22 @@
+function tests_octave()
+
+  %
+  % (C) Copyright 2024 CPP ROI developers
+
+  root_dir = getenv('GITHUB_WORKSPACE');
+
+  addpath(fullfile(root_dir, 'spm12'));
+  addpath(fullfile(root_dir, 'lib', 'bids-matlab'));
+  addpath(fullfile(root_dir, 'MOcov', 'MOcov'));
+
+  cd(fullfile(root_dir, 'MOxUnit', 'MOxUnit'));
+
+  moxunit_set_path();
+
+  cd(fullfile(root_dir));
+
+  initCppRoi();
+
+  run_tests();
+
+end
diff --git a/src/atlas/copyAtlasToSpmDir.m b/src/atlas/copyAtlasToSpmDir.m
index fbab32c..ad137dd 100644
--- a/src/atlas/copyAtlasToSpmDir.m
+++ b/src/atlas/copyAtlasToSpmDir.m
@@ -17,11 +17,6 @@ function copyAtlasToSpmDir(varargin)
 
   % (C) Copyright 2022 CPP ROI developers
 
-  if bids.internal.is_octave
-    % the atlas in the spm dir are only useful in matlab with the GUI
-    return
-  end
-
   args = inputParser;
 
   addOptional(args, 'atlas', 'AAL', @ischar);
diff --git a/src/atlas/extractRoiFromAtlas.m b/src/atlas/extractRoiFromAtlas.m
index 0783d86..455a64c 100644
--- a/src/atlas/extractRoiFromAtlas.m
+++ b/src/atlas/extractRoiFromAtlas.m
@@ -39,9 +39,9 @@
   args = inputParser;
 
   addRequired(args, 'outputDir', isChar);
-  addRequired(args, 'atlasName', @(x) isAKnownAtlas(x));
+  addRequired(args, 'atlasName');
   addRequired(args, 'roiName', isChar);
-  addRequired(args, 'hemisphere', @(x) ismember(x, {'L', 'R'}));
+  addRequired(args, 'hemisphere');
 
   parse(args, varargin{:});
 
@@ -50,6 +50,15 @@
   roiName = args.Results.roiName;
   hemisphere = args.Results.hemisphere;
 
+  if ~ismember(hemisphere, {'L', 'R'})
+    msg = sprintf('"hemisphere must be "L" or "R"": %s\nGot: "%s"', ...
+                  hemisphere);
+    bids.internal.error_handling(mfilename(), ...
+                                 'invalidHemisphere', msg, false);
+  end
+
+  isAKnownAtlas(atlasName);
+
   [atlasFile, lut] = getAtlasAndLut(atlasName);
 
   switch lower(atlasName)
diff --git a/src/atlas/unzipAtlas.m b/src/atlas/unzipAtlas.m
index ad36bd6..65295d9 100644
--- a/src/atlas/unzipAtlas.m
+++ b/src/atlas/unzipAtlas.m
@@ -59,7 +59,13 @@ function gunzipAtlasIfNecessary(file)
 end
 
 function gunzipWithOctave(file)
-  copyfile(file, [file '.bak']);
-  gunzip(file);
-  copyfile([file '.bak'], file);
+  if iscellstr(file)
+    for i = 1:numel(file)
+      gunzipWithOctave(file{i});
+    end
+  else
+    copyfile(file, [file '.bak']);
+    gunzip(file);
+    copyfile([file '.bak'], file);
+  end
 end
diff --git a/src/roi/thresholdToMask.m b/src/roi/thresholdToMask.m
index e46db5c..7e896e9 100644
--- a/src/roi/thresholdToMask.m
+++ b/src/roi/thresholdToMask.m
@@ -48,7 +48,7 @@
 
   % add peakThreshold and clusterSizeInfo to desc
   if ~isfield(bf.entities, 'desc')
-    bf.entities.desc = '';
+    bf.entities(1).desc = '';
   end
   descSuffix = sprintf('p%05.2f', peakThreshold);
   if clusterSize > 0
@@ -57,6 +57,10 @@
   descSuffix = strrep(descSuffix, '.', 'pt');
   bf.entities.desc = [bf.entities.desc descSuffix];
 
+  if isempty(bf.extension)
+    bf.extension = '.nii';
+  end
+
   bf = bf.update();
 
   hdr = spm_vol(inputImage);
diff --git a/tests/test_createRoi.m b/tests/test_createRoi.m
index f116dda..553ab81 100644
--- a/tests/test_createRoi.m
+++ b/tests/test_createRoi.m
@@ -9,7 +9,8 @@
 
 function test_createRoi_sphere()
 
-  volumeDefiningImage = fullfile(demoDir(), 'TStatistic.nii');
+  inputDir = setUpDemoData();
+  volumeDefiningImage = fullfile(inputDir, 'inputs', 'TStatistic.nii');
 
   sphere.location = [44 -67 0];
   sphere.radius = 5;
@@ -91,38 +92,20 @@ function test_createRoi_intersection_mask_sphere()
   value = fileparts(mfilename('fullpath'));
 end
 
-function value = demoDir()
-
-  value = fullfile(thisDir(), '..', 'demos', 'roi', 'inputs');
-
-  if exist(fullfile(value, 'TStatistic.nii'), 'file') == 0 || ...
-     exist(fullfile(value, 'visual motion_association-test_z_FDR_0.01.nii'), 'file') == 0
-    gunzip(fullfile(value, '*.gz'));
-  end
-
-end
-
 function  [roiFilename, volumeDefiningImage] = prepareRoiAndVolumeDefiningImage()
 
-  volumeDefiningImage = fullfile(demoDir(), 'TStatistic.nii');
-
-  roiFilename = fullfile(demoDir(), ...
-                         'space-MNI_atlas-neurosynth_label-visualMotion_desc-p10pt00_mask.nii');
-
-  if exist(roiFilename, 'file') == 2
+  inputDir = setUpDemoData();
 
-  else
+  volumeDefiningImage = fullfile(inputDir, 'inputs', 'TStatistic.nii');
 
-    zMap = fullfile(demoDir(), 'visual motion_association-test_z_FDR_0.01.nii');
+  zMap = fullfile(inputDir, 'inputs', 'visual motion_association-test_z_FDR_0.01.nii');
 
-    zMap = renameNeuroSynth(zMap);
-    zMap = resliceRoiImages(volumeDefiningImage, zMap);
+  zMap = renameNeuroSynth(zMap);
+  zMap = resliceRoiImages(volumeDefiningImage, zMap);
 
-    zMap = removePrefix(zMap, 'r');
+  zMap = removePrefix(zMap, 'r');
 
-    threshold = 10;
-    roiFilename = thresholdToMask(zMap, threshold);
-
-  end
+  threshold = 10;
+  roiFilename = thresholdToMask(zMap, threshold);
 
 end
diff --git a/tests/test_getPeakCoordinates.m b/tests/test_getPeakCoordinates.m
index 43502ee..c21b90e 100644
--- a/tests/test_getPeakCoordinates.m
+++ b/tests/test_getPeakCoordinates.m
@@ -10,9 +10,10 @@
 
 function test_getPeakCoordinates_basic()
 
-  roiImage = extractRoiFromAtlas(pwd, 'wang', 'V1v', 'L');
+  inputDir = setUpDemoData();
+  dataImage = fullfile(inputDir, 'inputs', 'TStatistic.nii');
 
-  dataImage = fullfile(demoDir(), 'TStatistic.nii');
+  roiImage = extractRoiFromAtlas(pwd, 'wang', 'V1v', 'L');
 
   reslicedImages = resliceRoiImages(dataImage, roiImage);
 
@@ -22,20 +23,4 @@ function test_getPeakCoordinates_basic()
   assertEqual(voxelCoord, [28 8 24]);
   assertElementsAlmostEqual(maxVal, 1.6212, 'absolute', 1e-3);
 
-  delete('*hemi-L_space-MNI_atlas-wang_label-V1v_mask.*');
-
-end
-
-function value = thisDir()
-  value = fileparts(mfilename('fullpath'));
-end
-
-function value = demoDir()
-
-  value = fullfile(thisDir(), '..', 'demos', 'roi', 'inputs');
-
-  if exist(fullfile(value, 'TStatistic.nii'), 'file') == 0
-    gunzip(fullfile(value, '*.gz'));
-  end
-
 end
diff --git a/tests/test_isBinaryMask.m b/tests/test_isBinaryMask.m
index c159148..877a356 100644
--- a/tests/test_isBinaryMask.m
+++ b/tests/test_isBinaryMask.m
@@ -9,50 +9,20 @@
 
 function test_isBinaryMask_true()
 
-  roiFilename = prepareRoiAndVolumeDefiningImage();
+  [roiFilename, zMap] = prepareRoiAndVolumeDefiningImage();
   isBinaryMask(roiFilename);
-
-end
-
-function test_isBinaryMask_false()
-
-  [~, zMap] = prepareRoiAndVolumeDefiningImage();
   assertExceptionThrown(@()isBinaryMask(zMap), 'isBinaryMask:notBinaryImage');
 
 end
 
-function value = thisDir()
-  value = fileparts(mfilename('fullpath'));
-end
-
-function value = demoDir()
-
-  value = fullfile(thisDir(), '..', 'demos', 'roi', 'inputs');
-
-  if exist(fullfile(value, 'visual motion_association-test_z_FDR_0.01.nii'), 'file') == 0
-    gunzip(fullfile(value, '*.gz'));
-  end
-
-end
-
 function  [roiFilename, zMap] = prepareRoiAndVolumeDefiningImage()
 
-  zMap = fullfile(demoDir(), 'space-MNI_atlas-neurosynth_label-visualMotion_probseg.nii');
-
-  roiFilename = fullfile(demoDir(), ...
-                         'space-MNI_atlas-neurosynth_label-visualMotion_desc-p10pt00_mask.nii');
-
-  if exist(roiFilename, 'file') == 2
+  inputDir = setUpDemoData();
 
-  else
+  zMap = fullfile(inputDir, 'inputs', 'visual motion_association-test_z_FDR_0.01.nii');
 
-    zMap = fullfile(demoDir(), 'visual motion_association-test_z_FDR_0.01.nii');
-
-    zMap = renameNeuroSynth(zMap);
-
-    threshold = 10;
-    roiFilename = thresholdToMask(zMap, threshold);
-
-  end
+  zMap = renameNeuroSynth(zMap);
+  threshold = 10;
+  roiFilename = thresholdToMask(zMap, threshold);
 
 end
diff --git a/tests/test_keepHemisphere.m b/tests/test_keepHemisphere.m
index 4efa9fa..d31446a 100644
--- a/tests/test_keepHemisphere.m
+++ b/tests/test_keepHemisphere.m
@@ -10,9 +10,7 @@
 
 function test_keepHemisphere_basic()
 
-  inputDir = fullfile(fileparts(mfilename('fullpath')), '..', 'demos', 'roi');
-
-  gunzip(fullfile(inputDir, 'inputs', '*.gz'));
+  inputDir = setUpDemoData();
   zMap = fullfile(inputDir, 'inputs', 'visual motion_association-test_z_FDR_0.01.nii');
 
   zMap = renameNeuroSynth(zMap);
@@ -32,8 +30,4 @@ function test_keepHemisphere_basic()
                     'file'), ...
               2);
 
-  % TODO check the data content
-
-  delete(fullfile(inputDir, 'inputs', '*.nii'));
-
 end
diff --git a/tests/test_labelClusters.m b/tests/test_labelClusters.m
index e634aa2..8deb498 100644
--- a/tests/test_labelClusters.m
+++ b/tests/test_labelClusters.m
@@ -9,7 +9,8 @@
 
 function test_labelClusters_basic
 
-  zMap = fullfile(demoDir(), 'visual motion_association-test_z_FDR_0.01.nii');
+  inputDir = setUpDemoData();
+  zMap = fullfile(inputDir, 'inputs', 'visual motion_association-test_z_FDR_0.01.nii');
 
   zMap = renameNeuroSynth(zMap);
 
@@ -19,7 +20,7 @@
   labeledClusters = labelClusters(zMap, peakThreshold, extendThreshold);
 
   expected = 'space-MNI_seg-neurosynth_label-visualMotion_dseg.nii';
-  assertEqual(exist(fullfile(demoDir(), expected), 'file'), 2);
+  assertEqual(exist(fullfile(inputDir, 'inputs', expected), 'file'), 2);
 
   labelStruct = struct('ROI', 'ns left MT', ...
                        'label', 1);
@@ -27,20 +28,6 @@
   roiName = extractRoiByLabel(labeledClusters, labelStruct);
 
   expected = 'space-MNI_seg-neurosynth_label-nsLeftMT_mask.nii';
-  assertEqual(exist(fullfile(demoDir(), expected), 'file'), 2);
-
-end
-
-function value = thisDir()
-  value = fileparts(mfilename('fullpath'));
-end
-
-function value = demoDir()
-
-  value = fullfile(thisDir(), '..', 'demos', 'roi', 'inputs');
-
-  if exist(fullfile(value, 'visual motion_association-test_z_FDR_0.01.nii'), 'file') == 0
-    gunzip(fullfile(value, '*.gz'));
-  end
+  assertEqual(exist(fullfile(inputDir, 'inputs', expected), 'file'), 2);
 
 end
diff --git a/tests/test_thresholdToMask.m b/tests/test_thresholdToMask.m
index cfa84aa..1b82404 100644
--- a/tests/test_thresholdToMask.m
+++ b/tests/test_thresholdToMask.m
@@ -1,5 +1,5 @@
 function test_suite = test_thresholdToMask() %#ok<*STOUT>
-  %
+
   % (C) Copyright 2021 CPP ROI developers
 
   try % assignment of 'localfunctions' is necessary in Matlab >= 2016
@@ -9,13 +9,10 @@
   initTestSuite;
 end
 
-% TODO refactor set up and teardown
-
 function test_thresholdToMask_default()
 
-  image = 'visual motion_association-test_z_FDR_0.01.nii.gz';
-
-  inputImage = setUp(image);
+  inputDir = setUpDemoData();
+  inputImage = fullfile(inputDir, 'inputs', 'visual motion_association-test_z_FDR_0.01.nii');
 
   peakThreshold = 5.0;
   outputImage = thresholdToMask(inputImage, peakThreshold);
@@ -24,15 +21,12 @@ function test_thresholdToMask_default()
   vol = spm_read_vols(spm_vol(outputImage));
   assertEqual(sum(vol(:) > 0), 2008);
 
-  teardown();
-
 end
 
 function test_thresholdToMask_cluster()
 
-  image = 'visual motion_association-test_z_FDR_0.01.nii.gz';
-
-  inputImage = setUp(image);
+  inputDir = setUpDemoData();
+  inputImage = fullfile(inputDir, 'inputs', 'visual motion_association-test_z_FDR_0.01.nii');
 
   peakThreshold = 5;
   clusterSize = 10;
@@ -42,23 +36,4 @@ function test_thresholdToMask_cluster()
   vol = spm_read_vols(spm_vol(outputImage));
   assertEqual(sum(vol(:) > 0), 1926);
 
-  teardown();
-
-end
-
-function inputImage = setUp(image)
-
-  rootDir = fullfile(fileparts(mfilename('fullpath')), '..');
-
-  gzImage = spm_file(fullfile(rootDir, 'demos', 'roi', 'inputs', image), 'cpath');
-  [~, basename] = spm_fileparts(gzImage);
-  gunzip(gzImage, pwd);
-
-  inputImage = renameNeuroSynth(fullfile(pwd, basename));
-
-end
-
-function teardown()
-  delete('*.nii');
-  delete('*.json');
 end
diff --git a/tests/utils/demoDir.m b/tests/utils/demoDir.m
new file mode 100644
index 0000000..aa8aa59
--- /dev/null
+++ b/tests/utils/demoDir.m
@@ -0,0 +1,9 @@
+function value = demoDir()
+
+  % (C) Copyright 2021 CPP ROI developers
+  value = fullfile(thisDir(), '..', '..', 'demos');
+end
+
+function value = thisDir()
+  value = fileparts(mfilename('fullpath'));
+end
diff --git a/tests/utils/rmRetinoAtlas.m b/tests/utils/rmRetinoAtlas.m
index c377f1e..a30c384 100644
--- a/tests/utils/rmRetinoAtlas.m
+++ b/tests/utils/rmRetinoAtlas.m
@@ -1,5 +1,5 @@
 function rmRetinoAtlas()
-  %
+
   % (C) Copyright 2021 CPP ROI developers
 
   pause(0.3);
diff --git a/tests/utils/setUpDemoData.m b/tests/utils/setUpDemoData.m
new file mode 100644
index 0000000..e83a030
--- /dev/null
+++ b/tests/utils/setUpDemoData.m
@@ -0,0 +1,16 @@
+function inputDir = setUpDemoData()
+
+  % (C) Copyright 2021 CPP ROI developers
+
+  inputDir = fullfile(demoDir(), 'roi');
+
+  tmpDir = tempDir();
+  copyfile(inputDir, tmpDir);
+
+  inputDir = tmpDir;
+  if bids.internal.is_octave()
+    inputDir = fullfile(tmpDir, 'roi');
+  end
+
+  gunzip(fullfile(inputDir, 'inputs', '*.gz'));
+end
diff --git a/tests/utils/tempDir.m b/tests/utils/tempDir.m
new file mode 100644
index 0000000..f502122
--- /dev/null
+++ b/tests/utils/tempDir.m
@@ -0,0 +1,6 @@
+function value = tempDir()
+
+  % (C) Copyright 2021 CPP ROI developers
+  value = tempname();
+  mkdir(value);
+end