|
From: | Mike Miller |
Subject: | [Octave-bug-tracker] [bug #51310] [octave forge] (signal) firls.m modification to include all 4 FIR types, Hilbert transformer, and differentiator |
Date: | Tue, 20 Mar 2018 22:28:41 -0400 (EDT) |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0 |
Update of bug #51310 (project octave): Status: None => Patch Submitted Release: 4.2.1 => other _______________________________________________________ Follow-up Comment #9: Thanks for the work you have done so far on this firls replacement. I have glanced at the function but I haven't looked in detail at the code yet. I have noticed a couple tiny things that could be fixed just by glancing at the code. 1. This function is basically a rewrite, there's no need to keep the Copyright line from the original function there is basically no original code left. 2. Please don't use narginchk, use the original form that calls the print_usage function. 3. The entire body of the function (between "function" and "endfunction") should be indented by 2 spaces. I haven't read enough here about the reason for the "MatlabCompat" option, but I would rather it be compatible by default and not print a message when it does this compatible operation. I gather that you find this compatibility a little distasteful. _______________________________________________________ Reply to this item at: <http://savannah.gnu.org/bugs/?51310> _______________________________________________ Message sent via/by Savannah http://savannah.gnu.org/
[Prev in Thread] | Current Thread | [Next in Thread] |