mattools / matGeom

Matlab geometry toolbox for 2D/3D geometric computing
BSD 2-Clause "Simplified" License
264 stars 97 forks source link

Standardization of the header #136

Closed oqilipo closed 10 months ago

oqilipo commented 1 year ago

Why? https://github.com/mattools/matGeom/commit/df42d708f9743142c0bc094c19fd68573c63006a#diff-ea7a74b46e3dfb9b3f4e15d0ff6f20ce90ad537c2d09defbf3747d93a4b94c1b

dlegland commented 1 year ago

In the recent versions of Matlab, the Contents Reports seems to work without explicitely providing function name in capital case. In other projects I started with alternative convention that directly starts with function description, making the H1-line somewhat shorter. I also started introduce changes in Matgeom. However, after some practice, I am finally not very convinced by the new convention... And in some cases, it is still necessary to use the capitalized function name (when first descirption word is the same as function name). So let's stick to the convention that was used until now...

oqilipo commented 1 year ago

I've added checkHeader.m. At the moment only the beginning of the first header line is checked and corrected. Additional checks could be added.

dlegland commented 1 year ago

yes, great!

oqilipo commented 1 year ago

How should the perfect author section of the header look like? ;-)


% ------
% Author: 
% E-mail: 
% Created: 
% Copyright 
dlegland commented 1 year ago

he he, yes, your proposition in fine. I used this for recent files, but there are still plenty of older files that do not follow this pattern.

I am not always very sure about copyright (affiliations may change, there may be multi-authors...). Not sure this is absolutely necessary also.

oqilipo commented 1 year ago

Hi David

The author section is standardized in most of the files except eight remaining. Run checkHeader for further information.

Some questions came up.

  1. Do we need the history section in some of the files? Git already writes a history for each file.
  2. The header section could be further standardized. However the standard has to be defined ;-), for instance:
    
    [...]
    % 
    %   Example
    %   example example example 
    %
    %   See also
    %   functionA, functionB, functionC

% ------ [...]



Kind regards
dlegland commented 1 year ago

Hi,

great work, thanks!

Do we need the history section in some of the files? Git already writes a history for each file. Clearly not. This was useful before more extensive use of source control, but this can now be removed.

The header section could be further standardized. However the standard has to be defined ;-), for instance: he he, yes! your proposal looks fine. I had some colons (":") at the end of "Examples" or "See also" sections, but they can be removed. Examples can be plural (often useful to have several). Additional section can be "References", but I use it only rarely.

I'll be out of office for few days, I try to check it back later

Best, DAvid

oqilipo commented 1 year ago

Maybe

[...]
% 
%   Example(s):
%   example example example
%
%   Reference(s):
%   - ReferenceA
%   - ReferenceB
%
%   To-do(s):
%   - To-do1
%   - To-do2
%
%   See also:
%   functionA, functionB, functionC

% ------
[...]

?

Kind regards

dlegland commented 1 year ago

I think we can remove the colons at the end. This will make things more look like section titles.

does the "(s") still make sections recognised with the help report?

I would remove the To-do list ; we can use GitHub for enhancements.

best!

oqilipo commented 1 year ago

With help report do you mean entering:

help functionName

?

Looks like this in MATLAB 2022b. grafik

dlegland commented 1 year ago

I mean the "Help Report" available as menu from the navigator helpReports

It provides some checkup, icluding the presence of "Example" and "See Also" fields. Also, when there is a ":" at the end, it does not always recognize it. helpReports2

The report also complains about Copyright, but I do not think it is necessary to change copyright every year. Did not find if there is an option to choose the fields/section to check (did really llok for...).

oqilipo commented 1 year ago

Oh, nice. I did not know this function.

Regarding "angleAbsDiff.m". I think to get recognized is has to be "See also " (with space at the end) and not "See also"

With the checkHeader function it would be easy to update the copyright every year to: Copyright 2003-2022

How you like?

dlegland commented 1 year ago

Oh, nice. I did not know this function.

I used it in the beginning, but it lacks more configuration. I am not sure this is still very useful, but at least this helps providing examples and links to other functions.

Regarding "angleAbsDiff.m". I think to get recognized is has to be "See also " (with space at the end) and not "See also"

oh, sometimes the analyzer is stupid... ok for a space at the end if it works better (haven't tested).

With the checkHeader function it would be easy to update the copyright every year to: Copyright 2003-2022

Yeah, complicated question... I am not a big fan of updating the copyright every year : this will make the git history of some files a collection of "updated copyright year" commits... But on the same way, this would help identify if the library version is totally outdated or not. So I agree for the "Copyright INITIAL_YEAR-CURRENT-YEAR" format.

oqilipo commented 1 year ago

Do you want to merge the current pull request before the next changes?

dlegland commented 1 year ago

yep, sorry... done!

oqilipo commented 1 year ago

Should I start updating the Copyright to 2XXX-2023? Do you want to have your E-mail standardized to "david.legland@inrae.fr" in all functions where it appears?

dlegland commented 1 year ago

yes, would be perfect, thanks!

oqilipo commented 1 year ago

I would remove the To-do list ; we can use GitHub for enhancements.

The checkHeader.m found: 'To-do(s)' found in: hexagonalGrid 'To-do(s)' found in: minDistancePoints 'To-do(s)' found in: squareGrid 'To-do(s)' found in: triangleGrid 'To-do(s)' found in: clipPolygon3dHP 'To-do(s)' found in: drawGrid3d 'To-do(s)' found in: adjacencyListToEdges 'To-do(s)' found in: drawDirectedEdges 'To-do(s)' found in: grRemoveMultiplePoints 'To-do(s)' found in: polyhedronNormalAngle 'To-do(s)' found in: cart2geod 'To-do(s)' found in: geod2cart 'To-do(s)' found in: medialAxisConvex

Do you want to open an issue (incl. enhancement label) for each file?

dlegland commented 1 year ago

I will have a look soon, I'm sure many of them are now deprecated...

dlegland commented 10 months ago

Hi, that was fixed in #https://github.com/mattools/matGeom/commit/ef587fc7dcb1f457efa05b357f016c6d040dc7f9, but I forgot to update this thread... I'm now closing!