washington-university / cifti-matlab Goto Github PK

Hi,
In the newest version of ciftisavereset I am able to open pconns, modify them to pscalar format, and the save as a pscalar file, but there appears to be a glitch.

`cii=ciftiopen('matrix.pconn.nii',workbench);
cii =

struct with fields:

metadata: [1×4 struct]
 diminfo: {[1×1 struct]  [1×1 struct]}
   cdata: [414×414 single]`

new=cii.cdata(:,414);
cii.cdata=new;
ciftisavereset(cii,'/filepath/new.pscalar.nii',workbench)

the file won't open in workbench. Also it cannot be read with wb_command -file-information On the original version it still works.

A different type of question, what should the formatting be for help information?

First, matlab help info of built-in functions is generally summary first, then multiple function signatures (really, using different optional arguments) with explanations interspersed. The current cifti-matlab help has function signatures first (and includes the function keyword), and then the description immediately below that, then options, and examples (when present) at the bottom.

Examples:

>> help unique
 unique Set unique.
    C = unique(A) for the array A returns the same values as in A but with 
    no repetitions. C will be sorted.    
   
    C = unique(A,'rows') for the matrix A returns the unique rows of A.
    The rows of the matrix C will be in sorted order.
...
>> help cifti_read
 function outstruct = cifti_read(filename, ...)
    Read a cifti file.
    If wb_command is not on your PATH and you need to read cifti-1
    files, specify "..., 'wbcmd', '<wb_command with full path>'".
 
    >> cifti = read_cifti('91282_Greyordinates.dscalar.nii');
    >> cifti.cdata = outdata;
    >> cifti.diminfo{2} = cifti_diminfo_make_scalars(size(outdata, 2));
    >> write_cifti(cifti, 'ciftiout.dscalar.nii');

How closely do people want this to match matlab builtin help formatting and order?

optional inputs

Some of the functions (generally lower-level) still have optional arguments that don't use varargin, so if we don't change them to use varargin, how should the help info look? Some ideas:

1. like matlab multi-signature help info on zeros:

    %function outmap = cifti_diminfo_make_scalars(nummaps)
    %function outmap = cifti_diminfo_make_scalars(nummaps, namelist)
    %function outmap = cifti_diminfo_make_scalars(nummaps, namelist, metadatalist)

2. square brackets, like many command line utilities:

    %function outmap = cifti_diminfo_make_scalars(nummaps[, namelist, metadatalist])

3. no notation, and expect people to read the help down to where it says what is optional (this is what is currently implemented):

    %function outmap = cifti_diminfo_make_scalars(nummaps, namelist, metadatalist)
...
    %   Only the nummaps argument is required.

optional outputs

Some functions also have multiple outputs, and in matlab all outputs are automatically optional (a function can be coded to check the number of output arguments and manually throw an error, or even change its behavior, but there is no reason to do that here). Can we expect users to be aware of this, or do we need some notation for that also? Example:

    %function [outdata, outroi] = cifti_struct_dense_extract_surface_data(cifti, structure, dimension)

The help info for unique starts with the 4 combinations of single vs multi output, and single versus multiple arguments, but discontinues that method as soon as it reaches more advanced options. So, some options:

1. like matlab help info on unique, start with a few combinations, then any further optional arguments use only the all-outputs notation:

    %function outdata = cifti_struct_dense_extract_surface_data(cifti, structure)
    %function [outdata, outroi] = cifti_struct_dense_extract_surface_data(cifti, structure)
    %function outdata = cifti_struct_dense_extract_surface_data(cifti, structure, dimension)
    %function [outdata, outroi] = cifti_struct_dense_extract_surface_data(cifti, structure, dimension)
    %function [outdata, outroi] = cifti_struct_dense_extract_surface_data(cifti, structure, dimension, somefutureopt)

2. separate input and output sections:

    %function outdata = cifti_struct_dense_extract_surface_data(...)
    %function [outdata, outroi] = cifti_struct_dense_extract_surface_data(...)
    %function ... = cifti_struct_dense_extract_surface_data(cifti, structure[, dimension, somefutureopt])

3. just the full outputs notation, and expect users to know how to ignore outputs they don't need (this is what is currently implemented)

Dear cifti-matlab experts,
When I use the read_gifti_file_standalone to read my xxx.shape.gii file, which is a metric file in HCP workbench version. There some error. the command is matrix=read_gifti_file_standalone('/xxx/xxx.shape.gii','data')
error:
Reference to non-existent field 'data'.
Error in read_gifti_file_standalone (line 48)
this.data{end+1} = gifti_DataArray(t,uid(i),filename);
I don't know what's wrong. When I execute structure=gifti('/xxx/xxx.shape.gii')
the return is:
val =
cdata: [32492x1 single]

Anyone can help me?

Best
Guoyuan

Given the rewrite of the library, this issue is to gather input on naming conventions, in the hopes of improving things before we declare the new library ready for users.

I see a likely source of confusion in cifti_dense_get_surface_mapping(), in that the string "surface mapping" may make users think it refers to "volume to surface mapping", which is a very different thing (resampling, and not merely a file format detail). Anyone have some ideas? A synonym like "correspondence" could work, but that is longer and more clunky, and I am drawing a blank on other good synonyms. Another possibility is to shorten them to "map" instead of "mapping" (cifti_dense_get_vol_all_map(), etc).

It would probably be good to standardize to using only one of vol or volume in the helper names, but I'm not sure whether clarity or brevity should be favored here.

If you have any other comments on the new library other than bugs, you can post them here, too (bugs deserve new issues).

@coalsont and @glasserm, I have deleted my copy of the repository, and updated the link on https://nl.mathworks.com/matlabcentral/fileexchange/56783-washington-university-cifti-matlab so that it now points to the WashU version.

You will still have to add yourself as contributors. After that, you can start documenting and implementing the changes that we discussed today.

@coalsont I mentioned some existing test scripts. You can find them here

https://github.com/robertoostenveld/fieldtrip/blob/master/test/test_bug2096.m
https://github.com/robertoostenveld/fieldtrip/blob/master/test/test_bug2734.m
https://github.com/robertoostenveld/fieldtrip/blob/master/test/test_bug2741.m
https://github.com/robertoostenveld/fieldtrip/blob/master/test/test_bug2893.m
https://github.com/robertoostenveld/fieldtrip/blob/master/test/test_bug2909.m

Especially the 2096 test script has a lot of cases. Please review them and let me know whether you want to have a copy of any of the data (which I'll share through a private channel).

This library does not return the color tables for Parcellation maps for reference datasets. The cifti file reports CIFTI_INDEX_TYPE_LABELS and the Matlab code dutifully reads the LabelTable Colors. However, the Matlab code never returns the color lookup table values to the user. Specifically, consider the XML

<Label Key="360" Red="0.478431" Green="0.145098" Blue="0.286275" Alpha="1">L_p24_ROI</Label>

which defines vertices of index 360 to have specific RGBA values and the name L_p24_ROI. While the Matlab code parses this XML table, it is not returned as part of the struct:

cii = ft_read_cifti('Q1-Q6_RelatedParcellation210.CorticalAreas_dil_Final_Final_Areas_Group_Colors.32k_fs_LR.dlabel.nii')

  struct with fields:

                 dimord: 'pos'
               indexmax: [64984×1 double]
                    hdr: [1×1 struct]
                   unit: 'mm'
         brainstructure: [64984×1 double]
    brainstructurelabel: {'CORTEX_LEFT'  'CORTEX_RIGHT'}
                    pos: [64984×3 double]