Re: New optics functions, was: Re: Contributing to octave-forge

[Carnë Draug]

On 18 January 2014 10:50, Martin Vogel <address@hidden> wrote:
> Am 18.01.2014 03:18, schrieb Carnė Draug:
>> On 16 January 2014 17:48, Martin Vogel <address@hidden> wrote:
>>> Dear Octavists,
>>>
>>> it took me a while to get going, but now, I just submitted the promised
>>> functions for optical calculations following the Mueller-Stokes and
>>> Jones formalism:
>>>
>>> https://savannah.gnu.org/patch/index.php?8278
>>>
>>> All three subdirectories of "inst/" must be on the path, then "demo
>>> mueller_stokes" or "demo jones" will give you some demonstration what
>>> the library is about. Further information can be found in Wikipedia,
>>>
>>> http://en.wikipedia.org/wiki/Mueller_calculus
>>> http://en.wikipedia.org/wiki/Stokes_vectors
>>> http://en.wikipedia.org/wiki/Jones_calculus
>>>
>>> I tried my best to have each function with its own documentation.
>>> As I plan to add further library functions (more elements, a different
>>> section for paraxial calculations using the famous ABCD-matrices, ...),
>>> the submitted code may serve as a nucleus to a new package "optics".
>>>
>>> I'd be happy to help as its maintainer, if there is a need for that job,
>>> but I'd need some initial help to set it up as such...
>>>
>>> Thank you all for an excellent job providing us with Octave,
>>>
>>> Martin
>>
>> Hi Martin
>>
>> I have created a repository with the basic structure for your package
>> [1]. I will need your sourceforge username to give you permissions to
>> push into it (or you can host it somewhere else and we can pull from
>> there). Until them, you can already pull it and commit your functions
>> (please follow the commit message guidelines [2]).
>>
>> Carnė
>>
>> [1] https://sourceforge.net/p/octave/optics/
>> [2] http://wiki.octave.org/Commit_message_guidelines
>>
>
> Hi Carnė,
>
> thank you, just sent my username by PM.

You should be able to push csets now.

> One more question: The canonical place to add directories upon package
> installation to the path is the PKG_ADD file, right? Or is there some
> automation built into the common Makefiles?

You should not need to do this. I just took a look into your package
and saw that you split your functions into 3 directories. I will very
very much strongly advise you against it. Just place them all in the
same directory. When you do this, you can also move the __foo__
functions you have in inst/misc/ into inst/private.

If you really wish to have them into separate directories, I would
then recommend to do it only during development, and to a bootstrap
script that moves the files from inst/jones and inst/mueller_stokes
down on level into inst/, and inst/misc into inst/private to do the
release. After all, once the pkg is installed, it should not make any
difference if the files are in the same or separate directories. But I
don't think it's worth the trouble.

Finally
* your copyright notices on each file say "This file is part of
Octave.". This will be a package in Octave Forge. You should remove
that line and replace other instances of Octave with ""this program"
* I also noticed that sometimes you do input check and return nothing
if there's no input arguments. I think it's much more standard to give
an error in incorrect usage (just call print_usage ) than to return
nothing (is there any function in Octave that requires input and
silently returns nothing when called with nargin == 0?)
* Don't leave "revision history" on the files. The revision history is
kept on the commit messages. If you want to have it present on a
release, create a Changelog file for the release.

Carnë