octave-patch-tracker
[Top][All Lists]
Advanced

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

[Octave-patch-tracker] [patch #8205] Addition to financial package: blk*


From: Carnë Draug
Subject: [Octave-patch-tracker] [patch #8205] Addition to financial package: blk*, bls*, and opprofit functions
Date: Tue, 08 Oct 2013 20:36:58 +0000
User-agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130917 Firefox/17.0 Iceweasel/17.0.9

Update of patch #8205 (project octave):

                  Status:                    None => Done                   

    _______________________________________________________

Follow-up Comment #1:

I have pushed your changes after editing the commit message (following the
commit message guidelines[1] but still keeping your signature) and adding the
new functions to the INDEX and NEWS files [2].

I have then made a few more changes[3] to follow Octave code guidelines:

0 use endif, endfunction, etc to end blocks rather than a general end;
0 replaced tabs per spaces;
0 moved the one sentence help text to the bottom (so that lookfor can be used
to find this functions);
0 used double quotes for strings;
0 always leaving a spaces after a function call (so we can index between
function call and variable indexing);
0 added a minimum input check (just checking the number of arguments);
0 set the variable defaults on the function definition line, rather than
checking if the variable name exists
0 if cheking if a condition is false, use "if (! condition)" and not "if
(condition == 0)"

On the subject of input checking would be great if you could extend it.

On blsimpv you use two for loops. This is likely to be slow. In addition, it
will only work up to 2 dimensions. And since Octave is column-major order, I
have swapped the order of it so it will process down each column.

Finally, could you write some tests for these functions? See for example, the
test blocks at the bottom of addtodate[4], datenum[5], or rgb2ntsc[5].

[1] http://wiki.octave.org/Commit_message_guidelines
[2]
http://sourceforge.net/p/octave/financial/ci/bb3ab896bc3319907dabc7e05d25164c2b17e76a/
[3]
http://sourceforge.net/p/octave/financial/ci/7350d2b4eaab53a26cfd933f19a5561a439c1f83/
[4]
http://hg.savannah.gnu.org/hgweb/octave/file/b43da3876b64/scripts/time/addtodate.m
[5]
http://hg.savannah.gnu.org/hgweb/octave/file/b43da3876b64/scripts/time/datenum.m
[6]
http://hg.savannah.gnu.org/hgweb/octave/file/b43da3876b64/scripts/image/rgb2ntsc.m

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8205>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

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