Fix Octave compatibility (as much as possible)

[jcohenadad]

Ref: brendenlake/omniglot@85a191b

@rtopfer what do you think?

[kousu]

There's several Octave compatibility issues. Can we make this issue a meta-issue to handle them all?

IMO this is a dependency for shimming-toolbox/shimming-toolbox#67.

[kousu]

1. Remove < matlab.mixin.SetGet. It does two things for us:

   1. Makes it a handle to make every object a pointer and cheap to pass around
   2. Provides this syntax: set(img,'Hdr',8), get(img,'Hdr') so you can access fields dynamically by a string name, which I don't think we're using, and anyway I want us to move away from mutable data anyway (Immutable Data Structures #71), and newer matlab has property setters which are pr
      So I think we don't need to be concerned with Gui implementation #2, and can achieve Generalizing shim programs #1 just with < handle

2. The documentation generator uses matlab-specific introspection.

3. Octave needs explicit dependencies:

   • octave -e 'pkg install -forge dicom'
   • and libgdcm needs to be installed separately on the system (brew install gdcm, pikaur install gdcm (it's not in Arch but it's in the AUR), apt-get install libgdcm2.8)
   • pkg load dicom at the top of every file that uses dicom ((I expect this to annoy Matlab, so it probably needs to be wrapped in an if octave ... or users can put this in ~/.octaverc (https://stackoverflow.com/a/38002280)

4. dicominfo(img).Manufacturer has an extra ' ' on the end in Octave, which seems silly. Maybe the matlab version calls strip() before returning?

   • Is there a strip() we could call? We need it for

   https://github.com/shimming-toolbox/shimming-toolbox/blob/39070d7c7eb7a2ee2edee8d91a83a07c700431a4/Img/misc/dicominfosiemens.m#L30

5. Octave is stricter about putting everything that should be in its own file in its own file:

   parse error near line 1 of file Ui/ShimUse.m
   
    classdef must appear inside a file containing only a class definition
   
   >>> classdef ShimUse < matlab.mixin.SetGet
   

6. parse_siemens_shadow is accessing things that Octave's dicominfo doesn't provide -- and really, shouldn't be. Can we do without it?

   error: structure has no member 'Private_0029_10xx_Creator'
   error: called from
       parse_siemens_shadow at line 29 column 12
       dicominfosiemens at line 31 column 42
       MaRdI at line 116 column 20
       example at line 13 column 5
   

[rtopfer]

As annoying as MATLAB is, I think we're stuck with it for now. Once a fully working prototype exists in MATLAB then it might be worth thinking about making it compatible, or translating it to python etc.

The presumed Toolbox user is someone with access to an MRI + shim hardware; in comparison, assuming they also have access to Matlab is an incredibly small requirement.

[kousu]

That's a fair response! It means we can't do #67, at least not online. We can make a testing suite that runs on lab computers though.

[jcohenadad]

The advantage of doing the move to Octave asap is to increase our testing coverage. Even if the project is not 100% fully-Octave compatible (but let's say, 80%), then this chunk can be included in our cloud-based testing framework.

[jcohenadad]

I would be curious to know how much Octave-compatibility we can get with the current framework.

[jcohenadad]

Can we make this issue a meta-issue to handle them all?

yes-- title changed

[jcohenadad]

We can make a testing suite that runs on lab computers though.

we have Azure pipeline set up internally, with Matlab license, for another project. Should we go that route?