patchGNU Octave - Patches: patch #8517, [octave forge] (signal)...

 
 

patch #8517: [octave forge] (signal) Legendre-Papoulis filter design functions

Submitter:  None
Submitted:  Tue 19 Aug 2014 03:35:52 AM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Postponed Privacy:  Public
Assigned to:  mtmiller Originator Email:  -email is unavailable-
Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 18 Sep 2014 04:15:03 AM UTC, comment #3: 

Well, it took more time than I would like. The files are changed according to your suggestions. I decided to rename them to 'legenp' and 'legenpap'.

Feel free to add other suggestions/request fixes (I'm not 100% sure of the proper formatting of the doc). Hopefully I'll be faster next time.

(file #32125, file #32126)

Anonymous
Wed 20 Aug 2014 03:36:26 AM UTC, comment #2: 


> Would you mind looking at some coding style fixups and adding unit tests?


Sure, I don't mind. Well, at the time I just blatantly copied the 'butter' function and change it to use 'legendap', as well as change its doc. Moreover I worked with a pretty old GNU Octave. So merely re-copying the 'butter' function should fix most coding style issues, and I'll fix the rest.

A bit unrelated, but I realized I forgot to add the warning about numerical accuracy, should I mention it in 'legendap' and refer to it in 'legendf'?

> Would you agree to rename these functions to "legendreap" and "legendref"? I think it would be clearer to distance these functions from the plotting function "legend".


I liked 'legendap' and 'legendf' as they are short names and reads somewhat well. 'legendreap' looks funny. On the other hand, this is how other filter design functions are named:

  • Butterworth: butter, buttap
  • Chebyshev type 1,2: cheby[12], cheb[12]ap
  • Elliptic: ellip, ellipap


In the same spirit, it could be 'legenp' and 'legenpap' (p for Papoulis). What do you think?

I realize also that there should be a 'legenpord' to match the 'buttord','ellipord',etc. functions. This will be a candidate of a future patch however.

Anonymous
Tue 19 Aug 2014 05:07:21 PM UTC, comment #1: 

Thanks for your contribution to Octave! This function can definitely be added to the signal package. Would you mind looking at some coding style fixups and adding unit tests?

It would be best for all new functions added to the signal package to adhere to GNU Octave coding style conventions. Here are some suggestions based on a quick review of your submission (based on a suggestions made for other recent submissions):

  • Use 2 spaces for indentation
  • Remove trailing spaces from all lines
  • Use endif instead of end
  • Use space before opening parenthesis on function calls, if, switch, etc, no space when indexing
  • If conditions: use parentheses around even trivial conditions
  • For loops: use parentheses on loop initializers
  • Keep parentheses on function calls with no arguments (e.g. print_usage ())
  • Use Texinfo format for public function documentation
  • Make sure to indicate the function name in error messages


Some simple particulars about this function:

  • Prefer lower case variable and inner function names, unless you want to differentiate matrices from scalars using uppercase local variable names


Can you write some tests for legendap as well?

Would you agree to rename these functions to "legendreap" and "legendref"? I think it would be clearer to distance these functions from the plotting function "legend".

Mike Miller <mtmiller>
Group Member
Tue 19 Aug 2014 03:35:52 AM UTC, original submission:  

Discussed here:

http://octave.1599824.n4.nabble.com/I-want-to-contribute-give-two-functions-to-the-signal-package-td4655971.html

More than a year after (oops!) I submit my code. I thought I did this before but apparently I didn't. Better late than never I guess.

These files should belong to the signal package.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #32126:  legenpap.m added by None (3KiB - text/x-objcsrc - Updated files)
file #32125:  legenp.m added by None (7KiB - text/x-objcsrc - Updated files)
file #31914:  legendf.m added by None (7KiB - text/x-objcsrc - Files to add to the signal package)
file #31915:  legendap.m added by None (3KiB - text/x-objcsrc - Files to add to the signal package)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-03-07 mtmiller Carbon-CopyRemoved 80942 -
    2018-03-30 mtmiller CategoryNone Forge : new function
        StatusNone Postponed
        Summarysignal package: Legendre-Papoulis filter design [octave forge] (signal) Legendre-Papoulis filter design functions
    2015-12-22 mtmiller StatusIn Progress None
        SummaryLegendre-Papoulis filter design (for signal package) signal package: Legendre-Papoulis filter design
    2014-09-18 None Attached File- Added legenp.m, #32125
        Attached File- Added legenpap.m, #32126
    2014-08-19 mtmiller StatusNone In Progress
        Assigned toNone mtmiller
    2014-08-19 None Attached File- Added legendf.m, #31914
        Attached File- Added legendap.m, #31915

    Back to the top

    Powered by Savane 3.14-3b9d.
    Corresponding source code