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

 
 

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

Submitted by:  None
Submitted on:  Tue 19 Aug 2014 03:35:52 AM UTC  
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Postponed Privacy:  Public
Assigned to:  Mike Miller <mtmiller> Originator Email:  -email is unavailable-
Open/Closed:  Open

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

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>
Project AdministratorIn charge of this item.
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 mtmiller (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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.3