octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: savevtk


From: c.
Subject: Re: savevtk
Date: Fri, 19 Nov 2010 09:37:24 +0100

On 18 Nov 2010, at 10:58, address@hidden wrote:

> 
> Message: 2
> Date: Wed, 17 Nov 2010 22:53:58 +0100
> From: Philip Nienhuis <address@hidden>
> Subject: Re: savevtk
> To: Levente Torok <address@hidden>
> Cc: address@hidden
> Message-ID: <address@hidden>
> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
> 
> Levente Torok wrote:
>> Dear Philip, Carlo and octave-dev,
>> 
>> I had a chance to polish things according to your commands.
> 
> Thanks, I'll have a look at them.
> 
>> However I have a few questions.
>> 
>> Many of the codes do not fit into this scheme but I can accept that
>> this is guideline for the future.
>> 
>>> - 2 spaces rather than tabs (but I still use tabs in my own scripts so I
>>> don't mind this particular one)
>>> 
>>> - spaces between internal function name and left paren; no space between
>>> array name and left paren or bracket
>>> 
>>> - functions end with "endfunction" rather than "return"
>>> 
>>> - appropriate end statements: if- elseif -else - endif / for - endfor /
>>> while - endwhile / etc.
>>> 
>> 
>> Why dont want we write code that maybe used with matlab too?
>> This would be a benefit for all I believe.
> 
> O sure, I believe that too, OTOH there are good reasons to keep Octave 
> style.
> For one, and also my main motive: Octave's script language is simply 
> superior to that of ML. (It's easy to write e.g., if...end%if and 
> for...end%for to retain ML compatibility but Octave's parser can more 
> reliably check code if we follow to the above style suggestions.)
> Other -probably more ideological- reasons have been brought up 
> repeatedly by Octave developers in the help-octave and 
> octave-maintainers mailing lists.
> 
> From a more practical point of view, code that is meant to be run 
> unchanged under ML doesn't seem to belong in the io package in the 
> "main" tree, but rather in some package in the "extra" tree.
> See here:
>    http://octave.sourceforge.net/developers.html
> section "Where does your code belong?"
> 
> Unless explicitly instructed otherwise with sufficiently convincing 
> reasoning I tend to stick to these guidelines. (Though I violated this 
> already by provisionally committing a first versions of your scripts to 
> svn, assuming updated and more compliant versions will follow.)
> 
>>> - spaces after commas in argument lists (e.g.,  "(a, b, c)" instead of
>>> "(a,b,c)")
>>> 
>>> - are you sure your function shouldn't return some value, for successful
>>> writing or not? I see no write error catching structures at all
>>> 
>>> - I think you should check for the return value of fopen and perhaps also
>>> fprintf to see if the calls succeeded (hint: fprintf returns the number of
>>> characters written).
>>> A try-catch or unwind_protect construct could be around the fprintf
>>> statements to maybe avoid dangling file pointers. Upon errors and ensuing
>>> break out of savevtk....() the output file will probably be closed anyway
>>> but personally I like more graceful solutions.
>>> 
>>> - comments start with # rather than %
>> Why is this if we support multiple type of commenting styles?
> 
> See above.
> 
>>> - in the Updates: section in the headers, please add some short description
>>> of what the update was all about
>>> 
>>> - The copyright sections in the headers seem incomplete
>>> 
>>> - in savevtk.m an empty line is needed between the copyright statement and
>>> the texinfo header
>>> 
>>> - the texinfo headers seem a bit odd to me. Have a look at e.g. the Excel or
>>> OpenOffice.org scripts for examples
>> 
>> Where are these?
> 
> In the io package. But if you use the octave-3.2.4 MingW binary for 
> Windows, it is easier to just do from Octave:
>   edit newfile.m
> and you'll probably have a nice template.
> 
>>> 
>>> - would a test case or some test section be useful?
>> I don't know the syntax of the example however it could be something like:
>> 
>> n=30;
>> X=zeros(n,n,n);
>> for x=1:n
>>   for y=1:n
>>     for z=1:n
>>       X(x,y,z)=1/sqrt( x*x + y*y + z*z );
>>     endfor
>>   endfor
>> endfor
>> Maybe someone can write the same code with matrix arithmetic I did
>> tackle with this.
>> 
>>> 
>>> But OK, maybe I am just too picky....
> 
> According to Carlo, to some extent I am.... :-) (see his last mail/post 
> to me)
> 
> If you want your scripts in the io package I could adapt the code 
> (-style) a bit further for you (and send them to you for review before 
> committing them), but if you want to retain ML compatibility I suppose 
> your scripts perhaps should rather be elsewhere in the svn trunk.
> 
> Let me know what you want.
> 
> 
> Thank you Levente.
> 
> Philip

As this thread is about functions for Octave-forge packages I think it should 
be continued on the OF mailing list,
so I am moving it there.
c.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]