I have had a close look at the files you linked in comment #14. Thanks for preparing them! Here are some comments and wishes for improvements:

general:

• As mentioned before, I would prefer to have unit tests that also work locally, without the need to download a sample file from the internet.
• There were some files in the github repo, that are unnecessary for an inclusion into the image package. I have just ignored them: LICENSE, README, .miss_hit . I just looked at the remaining 8 m-files.
• All error message strings should start with the name of the function that issues this error message. Please also mention which if the input arguments had a problem, if possible. For examle "myfunc: input I must be positive" instead of "input must be positive".
• There are always examples in the help string. (This is nice.) Please add a short text like "example code:" directly in front of these examples, so a user can see that what follows is an example.
• Please also use "endif" instead of "end" at the end of a "if" section. This is Octave coding style. (You can just search for "end" in the code, there should not be any.)
• Please put a single space character between a function name and its arguments, whenn calling a function, e.g. "disp (5)". But do not put this space sign when indexing a matrix, e.g. "M(5)".
• 5 of the 8 m-files are helper functions. Those do not exist in Matlab. I would suggest to make them private functions, e.g. by placing them in the "private" subfolder of the "inst" code in the image repository. This way you do not need to care about the user interface during future code changes. This way it is also acceptable (in my opinion) to not have any unit tests in those functions. An alternative would be to add several unit tests to all those helper functions and then leave them around as "normal" user functions. Just let me know how you would like to proceed here, I can the adapt this once we push your new functions to the image repository.
• At some places, there are checks for OCTAVE_VERSION in the code (I found this in nii2jnii.m and in niftiwrite.m). As part of the Octave image package this is unnecessary. It would be better to remove these checks.
• The 3 main functions (niftiinfo, niftiread, niftiwrite) should have a lot more unit tests.  Generally those tests should:
  • cover all possible input syntax versions (and also some not-possible versions to check for error messages)
  • check all major use cases of the function, also all possible file types the function is ment to deal with (e.g. zipped or non zipped)

comments on individual functions:

• niftiinfo:
  • When I run the demo code in this new function under Matlab, then I get slightly different results. I cannot judge which result is "correcter" in this case:
    • spaceUnits = 0 (Matlab: 'unknown')
    • timeUnits = 0 (Matlab: 'None')
    • slicecode = 0 (Matlab: 'unknown')
    • transformname = Old (Matlab: 'None')
  • The data type of header.ImageSize is "double" in Matlab and uint16 in this code.
• nii2jnii: In the error message on line 101, it is not necessary to mention "Octave to be installed" if the code becomes part of an Octave package.
• nitfiread: nothing additional to the general comments
• nitfiwrite:
  • When I run the last demo code from the Matlab help page here: https://de.mathworks.com/help/images/ref/niftiwrite.html and change the info.Description property to something new, then niftiwrite, and afterwards niftiread this. Then as a result I cannot find my individual Description property in the read-in result. Is this correct?

I would be happy to eventually include this code into the image package. And I hope the above comments help you to adapt your code for this purpose. (The biggest work package is probably to add significantly more unit tests to the three user visible functions. But this is important to help the code stay usable in spite of a lot foreign future code changes in the image package as well as in core Octave. So please invest this effort to make your code "ready for future", no-one can do this better than you.)

I just committed an initial set of units with octave-styled docstrings at

https://github.com/NeuroJSON/jnifty/tree/octave9853

Please let me know if you see anything that you want me to tweak - one question I have is that some of the units requires nifti data files to do demo/tests, I am currently using

urlwrite('https://nifti.nimh.nih.gov/nifti-1/data/minimal.nii.gz', [tempdir 'minimal.nii.gz']);

to download a standard sample to tempdir, but this requires Internet connection, and it seems to be slow to download.

if we don't have to use official sample, I can also locally generate sample files, such as

https://github.com/NeuroJSON/jnifty/blob/octave9853/nii2jnii.m#L60-L63

let me know what you think. I ran some quick tests on octave 4.2.2 (Ubuntu 18.04) and seems to be working ok

just to let you that I have restarted working on code style reformatting.

the latest version can be found at

https://github.com/NeuroJSON/jnifty/tree/octave9853

the files included are the minimal set as discussed in comment #5, where I added fopen('z') and removed zmat/jsonlab dependencies (although, if they are installed, they can also be called).

I am using miss_hit (https://github.com/florianschanda/miss_hit) in the octave mode (--octave) to do as much automated formatting as possible. To tune the miss_hit config rules, I would like to get some pointers on the formatting style

So far, the current miss_hit formatting rules have done the following:

1. set indentation to 2 spaces
2. adding white spaces after comma and around operators

I might have to do some of the remaining formatting manually. Before that, I want to know the following:

Which of the styles in https://wiki.octave.org/Octave_style_guide are absolutely required? is endfunction/endif/endfor required? is using '#' instead of '%' required? if someone can point me to a template file, that would be very helpful. thanks

comment #9:

> I'm a bit worried that adding this would add too many dependencies to the image package when at the moment it has none. Also, since I feel this is a quite specialised topic, a separate nifti package may be more suitable.

hi Carnë, sorry for the delay - does the dependencies listed in comment #5 acceptable? while I am enthusiastic on the JSON/JNIfTI format, I am happy to just make the plain nifti support available to octave - and make it compatible with matlab.

hi Carnë, you can find .nii or .nii.gz files in many websites,

for example

https://github.com/neurolabusc/MRIcroGL/tree/master/Distro
https://nifti.nimh.nih.gov/pub/dist/data/nifti2/

I also included a few my tools for testing purposes

https://github.com/fangq/jnifti/tree/master/samples/headct
https://github.com/fangq/brain2mesh/tree/be391ecf1d225e9b5b4a4b0cab2078a055774ddd/examples/SPM


I am happy to convert my additional toolboxes into octave-forge packages. I just need someone to point me to a document regarding the formats for proposing a new octave-forge package.

to optionally support JNIfTI (JSON/JData based nifti files), my functions requires JSONLab - not just jsonencode and jsondecode, but jdataencode/jdatadecode as well as binary JSON (UBJSON) support.

currently, jsonencode/decode was discussed in https://savannah.gnu.org/bugs/?53100, which does not handle encode/decode JData formats (http://github.com/fangq/jdata).

in the following week, I may have some free time during the evenings of a conference, so I will start working on formatting the proposed units into Octave required comment formats. I will post those when I am done.

I do like the proposal from comment #5. (Assuming that it is still a significant benefit to have a niftiread function without any json support, which I cannot judge at all.)

@Guillaume: If you are willing to take on the burden of adapting Qianqian's code as outlined in comment #5, I think this could probably be a good extension of Octave's image package. Please ask here when any implementation questions come up while doing this.

@Qianqian: I am not a maintainer of the image package and I am aware of the underlying context your work is built upon but I do not think the niftiread/write/info implementations in Octave should require any extra dependencies. JSON is currently nowhere in the NIfTI specifications and for compression, one could take advantage of the "z" mode for fopen() in Octave (something that Matlab does not have):
https://octave.org/doc/v5.1.0/Opening-and-Closing-Files.html
I would be happy to help reformatting your code if needed.

Thanks you for the intention to contribute code to the Octave image package. You are very welcome as a first time contributor!

Those nifti-xxx functions were introduced to the Matlab image processing toolbox with version R2017b. So in general it would be very nice to have them in Octave as well.

I have personally never used a nifti file so far, nor am I an expert in software development, including the libraries you mentioned. (So some of the following questions might not sound very clever. I do know something about image processing, though...)

Here are some questions that come to my  mind:

• Are all of the contributed files written by YOURSELF? And could you "donate" them with a GPL compatible license? (I have seen an Apache license in nii2jnii.m for example, is it GPL compatible? Or could you re-license it under GPL?)

• Could you list the necessary LIBRARIES (toolboxes) that your code depends on in more detail? I have already understood the following dependencies:
  • JNIfTI toolbox
  • ZMat toolbox (for dealing with compression?)
  • JSONLab toolbox (for dealing with Json?)
  • any more tolboxes or library necessary?

• Are you the author of all of the above toolboxes? Are those all available under a GPL compatible license (I have seen GPLv3 at many places in the mentioned toolboxes)

• The octave image toolbox is currently "self contained" in the sense that it does not require to install any formal dependencies beforehand. So all code that your new functions need from the mentioned toolboxes would probably also need to be included into the Octave image toolbox. This is the reason behind the next question:
• Would we need to include ALL CODE of the above toolboxes?
  • Or is some of the functionality already implemented in Octave and can be used instead? (Octave can already deal with compressed files I think. Is this functionality not good enough for your intended use?)
  • Are some functions of those toolboxes NOT used by your nitfi functions and could therefore be omitted from the inclusion into the Octave image package?

• Another idea: In the case that (a) also those libraries are your own code and (b) they can be licensed with GPL and (c) they add functionality regarding compession and/or json functionality to Octave that isn't there yes, it might be worth discussing with "core Octave" about including these parts of your code there. But in this case those libraries should probably mimic Matlab compatible behavior (e.g. Matlab's jsondecode.m ...). This might be more than you intended to do.

• In some toolboxes there seem to be pre-compiled functions (I have seen zipmat.mex for exmaple). Are those files also available as C source code (under GPL)? Are they coded in a way that Octave can compile them on its own?

• In general there is a coding style for "core Octave", regarding C-code and m-code as well. It can be found here:
  • https://wiki.octave.org/Octave_style_guide
  • https://wiki.octave.org/C%2B%2B_style_guide

Please regard my above questions as expression of my interest into your code.

Maybe someone with more knowledge about C-code and library inclusion policies in core Octave could also leave some comments here?

@Carne: Could you comment on this, maybe?

@Qianqian: If you don't get a response on this bug tracker during the next weeks, it might be worht leaving a short note (mostly pointing to this bug tracker issue) on the mailinglist: -email is unavailable- .

I would like to contribute the below missing functions to octave-image

- niftiread
- niftiinfo
- niftiwrite

The current version can be found at
https://github.com/fangq/jnifti/tree/master/lib/octave

and are also attached in this thread. They can handle .nii, .nii.gz, .hdr/.img, .hdr.gz/.img.gz files. The input/output of these function smatch MATLAB's versions.

The newly drafted functions do require a set of helper functions to fully function:

- To read/write un-compressed NIfTI-1/2 (.nii) files, it minimally needs the JNIfTI toolbox at https://github.com/fangq/jnifti/tree/master/lib/matlab

- To read/write gzip-compressed NIfTI-1/2, it needs the ZMat toolbox at http://github.com/fangq/zmat

- To convert the NIfTI data to the more portable version for easy manipulation, i.e. the JNIfTI format, it needs the JSONLab toolbox at http://github.com/fangq/jsonlab

I've tested these functions on both Octave 4.2 and 5.1, using the NIfTI-1/2 sample data provided at

https://nifti.nimh.nih.gov/nifti-1/data

all works nicely.

I am wondering if anyone is interested in reviewing these functions and give some suggestions? first time contributor to Octave.