Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[STAPLE+GIBOC] Documentation software #60

Open
modenaxe opened this issue Jun 9, 2020 · 24 comments
Open

[STAPLE+GIBOC] Documentation software #60

modenaxe opened this issue Jun 9, 2020 · 24 comments
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@modenaxe
Copy link
Owner

modenaxe commented Jun 9, 2020

I found this toolbox that generates documentations: https://www.artefact.tk/software/matlab/m2html/

@modenaxe modenaxe added the documentation Improvements or additions to documentation label Jun 22, 2020
@modenaxe modenaxe changed the title [STAPLE] Documentation [STAPLE] Documentation software Jun 22, 2020
@modenaxe
Copy link
Owner Author

I close issue #59 about documenting GIBOC-core as it is relevant to this issue. @renaultJB

@modenaxe modenaxe changed the title [STAPLE] Documentation software [STAPLE/GIBOC-core] Documentation software Nov 15, 2020
@modenaxe modenaxe pinned this issue Nov 15, 2020
@modenaxe modenaxe added this to the JOSS paper milestone Nov 15, 2020
@modenaxe modenaxe changed the title [STAPLE/GIBOC-core] Documentation software [STAPLE+GIBOC] Documentation software Nov 15, 2020
@renaultJB
Copy link
Collaborator

I've read the documentation of m2html. Correct me if I'm wrong, but there is no fix standard to which the in-script documentation must conform. So, @modenaxe, when adding/updating the function documentation to GIBOC should I follow this standard ?

% PLOTBONELANDMARKS Add point plots, and if required, labels to bone
% landmarks identified throught the STAPLE analyses.
%
%   plotBoneLandmarks(BLStruct, label_switch)
%
% Inputs:
%   BLStruct - a MATLAB structure with fields having as fields the name of 
%       the bone landmarks and as values their coordinates (in global
%       reference system).
%
%   label_switch - a binary switch that indicates if the bone landmark
%       names will be added or not (as text) to the plot.
%
% Outputs:
%   none - the points are plotted on the current axes.
%
% See also PLOTDOT.
%
%-------------------------------------------------------------------------%
%  Author:   Luca Modenese
%  Copyright 2020 Luca Modenese
%-------------------------------------------------------------------------%
function plotBoneLandmarks(BLStruct, label_switch)
    BLfields = fields(BLStruct);
    for nL = 1:numel(BLfields)
        cur_name = BLfields{nL};
        plotDot(BLStruct.(cur_name), 'k', 7)
        if label_switch==1
            text(BLStruct.(cur_name)(1),...
                BLStruct.(cur_name)(2),...
                BLStruct.(cur_name)(3),...
                ['    ',cur_name],...
                'VerticalAlignment', 'Baseline',...
                'FontSize',8);
        end
    end
end

@modenaxe
Copy link
Owner Author

@renaultJB I am still trying to figure this out for the following reasons:

  1. m2html seem pretty basic, i would like to have a documentation that looks "cooler", like this page from the GIbbon documentation and it's obtained using this link
  2. the format above is the standard for Matlab functions if you type help name_func in the matlab command line, which is why I used it as much as possible as temporary solution.

I propose the following: document using the format above, and if necessary I will update it when I come up with a good solution, would that be ok?

@modenaxe
Copy link
Owner Author

by the way, for running m2html I had to modify TriDistanceMesh.m which had some imbalanced brackets. I don't think the function is ever used in any algorithm, but @renaultJB could you double check that I haven't messed up anything?

@renaultJB
Copy link
Collaborator

Indeed, TriDistanceMesh.m is not used anywhere. The code doesn't make much sense I think it was not terminated. The original goal was to find the signed distances between points from a point cloud and a triangulation object. It's far from doing that for now.
As it is not used anywhere, deleting it from STAPLE might be the best option, it will still be available at the GIBOC repo (in case of).

@renaultJB
Copy link
Collaborator

About the comment format, that works for me I will create a branch from master and do the the comments on GIBOC-Core there

@modenaxe
Copy link
Owner Author

great! I have removed the function in the meanwhile

@renaultJB
Copy link
Collaborator

renaultJB commented Nov 22, 2020

I've started to comment triangulation functions see this commit. Do not hesitate to tell me if the comments are unclear.

@modenaxe
Copy link
Owner Author

you're going like a boss! 😄

@modenaxe modenaxe unpinned this issue Mar 4, 2021
@renaultJB
Copy link
Collaborator

@modenaxe There is still a lot I have to do in terms of commenting, but before continuing I wanted to let you know that I found a contribution to sphinx that makes it work with Matlab files https://github.com/sphinx-contrib/matlabdomain.

I've run some tests and it seems to work correctly, as you can see for yourself https://htmlpreview.github.io/?https://github.com/modenaxe/msk-STAPLE/blob/giboc_core_with_comments/_build/html/index.html (FYI, I had to wait 1 to 2 minutes to get proper html/css rendering on this link.

In order to test, I had to make a few adjustments and put the functions docstrings inside the function. I don't think there is a way around that. I could write a Python Function to convert previously written docstrings to the required format.

@modenaxe
Copy link
Owner Author

@renaultJB this is very interesting. And if I remember well sphinx is very custumizable as well and we could create an organization for STAPLE and host the documentation on github pages, like here.

@modenaxe
Copy link
Owner Author

@renaultJB I think it's worth giving a try. Could you maybe try this on a branch and we see how it looks like when most of the toolbox is converted?

@renaultJB
Copy link
Collaborator

Ok, with sphynx we a have a choice between two main types of Docstrings format ; Google Style vs. Numpy Style. I've just tested them, and they both works with matlab code.

Some examples copy pasted from the provided link.

Google style:

def func(arg1, arg2):
    """Summary line.

    Extended description of function.

    Args:
        arg1 (int): Description of arg1
        arg2 (str): Description of arg2

    Returns:
        bool: Description of return value

    """
    return True

NumPy style:

def func(arg1, arg2):
    """Summary line.

    Extended description of function.

    Parameters
    ----------
    arg1 : int
        Description of arg1
    arg2 : str
        Description of arg2

    Returns
    -------
    bool
        Description of return value

    """
    return True

What do you think @modenaxe ?

@renaultJB
Copy link
Collaborator

renaultJB commented Mar 20, 2021

Also something to be noted, in the matlab sphinx contrib was specifically designed to ignore before function comments, so we will certainly have to move the docstrings inside the functions.

@renaultJB
Copy link
Collaborator

Ok after doing some tests and documenting myself. The numpy style appears to be more suited to our needs as it allows natively the description of multiple return values (which is the case for most of our functions).

@modenaxe
Copy link
Owner Author

@renaultJB thank you for testing! My only objection to this is that we will need to change ALL function documentation, which is a relatively big task. Out of curiosity, have you had a look at the MATLAB options I listed above? One of those would also allow to distribute the toolbox as a proper MATLAB package, which could be nice...

@renaultJB
Copy link
Collaborator

Yes, I had anticipated this issue. I'am writting a python module to do the conversion between already written matlab docstrings and numpy docstrings. I've read the matlab options but I don't find the explainations very clear.

Having a proper toolbox would be great for ease-of-use. Maybe we could do a minimal within toolbox documentation that refers to an online (and/or static?) html documentation ? I think it might be possible to link the sphinx doc within the matlab toolbox, but it will not have a natural matlab feel at all.

Also the Matlab command help staple_function works with numpy or google docstrings provided that there is no other comments before function definition.

What do you think ?

@modenaxe
Copy link
Owner Author

Ahhh I didn't know that the help header worked also below the function name, although I tend prefer it above for readability.
If you have the time of giving a try, let's go for sphynx. Using the MATLAB Markdown implies rewriting a lot of documentation as well, so whatever the choice we will have to work on it.

@renaultJB do you know if then there is some way of automate the doc generation, e.g. we push some new function and the documentation gets updated via GitHub actions? That would be a game changer for sustainability of the repo.

@renaultJB
Copy link
Collaborator

Yes, I agree that docstrings before code is more readable especially in case of large docstrings.

I've made some tests and developed a python object to generate docstring template numpy style for undocumented function and the object also try to convert pre-existing matlab docstrings.

I've made an example that display both original matlab docstrings and automatically generated numpy style one inside the code. @modenaxe let me know if that seems ok ?

There are temporary place holder for fields that cannot be easily infered from matlab docstrings (eg. __TYPE__) that will have to be replaced manually.

@modenaxe
Copy link
Owner Author

modenaxe commented Apr 1, 2021

@renaultJB if the script you used in some branch? I can have a look, the result looks promising

@renaultJB
Copy link
Collaborator

@modenaxe yes i have all committed to a branch named sphinx_documentation. The python scripts are in the docs folder at the root there are still under heavy development ^^.

HTML files are now in the repo also new example. It is still very basic, and i still have to learn how to make a proper table of content.

On GIBOC_core (that i had to rename from GIBOC-core to make It compatible with sphinx), I am a this point :

  • FittingFun
  • GeomtricFun
  • TriangulationFun (I've a draft of the comments but I need to properly format them)
  • MeshReadFun
  • PlotFun

@renaultJB
Copy link
Collaborator

renaultJB commented Apr 7, 2021

Update :

  • FittingFun
  • GeomtricFun
  • TriangulationFun
  • PlotFun
  • MeshReadFun

@modenaxe
Copy link
Owner Author

modenaxe commented Apr 9, 2021

Ok, next week I will start commenting/double checking the algorithms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants