[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.