On Wed, Jun 21, 2017 at 1:47 PM, Nicholas Jankowski <
address@hidden> wrote:
> On Wed, Jun 21, 2017 at 1:25 PM, je suis <
address@hidden> wrote:
>> Right, I hope I covered most of the possibilities, the script is
>> updated in
https://github.com/archbugaboo/Octave_signal. I've also
>> modified/corrected other checks in there, I hope it's more robust.
>> Should I also add some examples? There are some in the description,
>> but should they also be added with the %! line beginning? And, if
>> all's well, should I use
bugs.octave.org now?
>>
>> Vlad
>
> trying your first example, I still get:
>
>>> h = firls(30, [0 0.3 0.4 1], [0 0 1 1])
> error: 'k2' undefined near line 367 column 19
> error: called from
> firls at line 367 column 10
So, yes, I think you should put a few simple examples in your self tests. It doesn't need to be exhaustive, but you want to make sure output doesn't break. The examples are fine for that. If there are simple, or even trivial, cases, those may be easier to write simple tests for.
Generally for tests, I like to have both an error test to go with each input check and a testcase for any major input options or code branches. This is where you try to break your function on purpose. For example, I see you have a narginchk and that's covered by your error tests, but a length(N) check that doesn't get tested by any error tests. similarly, you don't have a test for non-integer N, nor any self-test.
>> firls (2.1, [0 1], [0 1])
warning: non-integer range used as index
warning: called from
firls at line 290 column 3
error: q(2.1): subscripts must be either integers 1 to (2^31)-1 or logicals
error: called from
firls at line 290 column 3
this isn't called until the call to toeplitz.
then, prior to any upload running a 'test firls' will let you know things are still behaving.
At this point I'd also recommend naming the file firls.m. it is going
to be used to replace firls and as long as you have it saved in your
working directory Octave will always use that version first. And feel free to create a bug tracker entry. call out that the current firls is deficient in whatever ways, give a reference link to the mailing list conversations, a link to your github, etc. cc the package maintainer on the bug report (Mike), then when it gets to the point that the function is about ready to be submitted we can generate a patch to do so.
fixing any obvious bugs (k2?), checking output against examples & matlab, adding tests, and maybe revising a bit to match Octave's preferred 'coding style' [1], should about cover it.