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

 
 

patch #9589: [octave forge] (signal) pow2db: new function

Submitter:  P Sudeepam <sudeepam>
Submitted:  Mon 12 Mar 2018 01:24:54 PM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  mtmiller Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 10 Apr 2018 05:34:53 PM UTC, comment #9: 

I added db2pow and pow2db to the signal package by combining your two patches into one:

https://hg.mtmxr.com/octave-signal/rev/0cd7ec2d2509

I made coding style and doc string changes, and added the new functions to the NEWS and INDEX files:

https://hg.mtmxr.com/octave-signal/rev/27289d5ac615

Please take a look at the changes I have made to help with your future submissions.

Mike Miller <mtmiller>
Group Member
Sat 31 Mar 2018 04:13:23 PM UTC, comment #8: 

Thank you for pointing out these little things. They would help me understand the coding standards better.

Kindly allow me a week's time to finish this work. I'm busy with my mid-term examinations these days and hence the delay.

P Sudeepam <sudeepam>
Sat 31 Mar 2018 06:03:42 AM UTC, comment #7: 

Can you please combine the series of patches you have posted here into a single patch? You can do this using hg histedit or hg fold, or other non-hg patch management tools.

Please remove the %!demo block at the end, it isn't offering much that the %!tests don't already show (a user can run "test pow2db verbose" to see the test specifics).

The error() message should be formatted like other error messages you see in Octave or in the signal package. Specifically, something like "pow2db: POW must be non-negative real values".

Have you checked that the test for less than zero works when the input is an array with some negative and some positive values? Can you add that as a test case to the "Test input validation" section?

Have you tried the doctest function yet? Can you install doctest (pkg install -forge doctest) and run "doctest pow2db" and see if you can rewrite the doc string example to pass?

And lastly, can you keep your name in mixed case on the Copyright line, just as you have it in the hg commit log, don't include your email address in the Copyright line, and can you use the most recent revision of the GPL license header block? You can find it in any of the other .m files in Octave or in the signal package.

I realize and admit that these are a lot of nitpicky things, but these are things that I would have to be going through and fixing on my own if you don't help me with them, so it would be helpful and good practice if you wouldn't mind helping me take care of them.

Mike Miller <mtmiller>
Group Member
Thu 15 Mar 2018 12:25:29 PM UTC, comment #6: 

Mike Miller,can you give the link of the total unimplemented functions list?
like you gave https://wiki.octave.org/Signal_package#Missing_Functions.

jack Bonz <vitalityz>
Tue 13 Mar 2018 06:07:57 PM UTC, comment #5: 

I would be glad to look at the existing patches.

As for the new functions, should I ping you when I decide which ones I want to implement so they could be allotted to me first, or should I implement them and submit the related patches directly?

Also, can I submit edits to the wiki page to point out the current status of the functions, such as 'under review', 'reviewed and ready to merge' etc. This, I believe, would help other people who would be wanting to contribute.

P Sudeepam <sudeepam>
Tue 13 Mar 2018 04:33:21 PM UTC, comment #4: 

Thanks, please take care of those things.

There is a list of missing functions in the signal package at https://wiki.octave.org/Signal_package#Missing_Functions.

Some of those missing functions already exist on this patch tracker. It would be very helpful if you would look at existing patches and help test them or review them. Some have been reviewed and are ready to be merged, some have not yet been looked at.


Mike Miller <mtmiller>
Group Member
Tue 13 Mar 2018 01:05:12 PM UTC, comment #3: 

Thankyou Mike. I will do exactly what you have said. I will use the GPLv3 license and try to make the code adhere to Octave coding standards better. My full name is Sudeepam Pandey, however, my legal name is P Sudeepam so I suppose I should be using this name in the copyright line and commit log like I already have done?

I can check what MATLAB does since I have it installed on my university’s computers.

Yes, I will be able to configure my code editor for unix like endings.

I'll improve the db2pow and pow2db functions, keeping these points in mind. Owing to my background, I would also like to implement other missing functions of the Signal package. If you see it fit and an exact list of missing functions exists then I could look at it and decide which functions I would be able to implement and ask you to assign those to me. If a list does not exist, then I will be happy to put one together. I'd be thankful if you could guide me on this.

P Sudeepam <sudeepam>
Mon 12 Mar 2018 06:43:10 PM UTC, comment #2: 

Do you have a way to configure your code editor for unix line endings?

Mike Miller <mtmiller>
Group Member
Mon 12 Mar 2018 06:16:42 PM UTC, comment #1: 

Thank you for your submissions for db2pow and pow2db. These functions are needed in the signal package and have not yet been implemented. The code looks mostly correct and the functions are doing what they should be doing.

I will have some further suggestions and style improvements, but I encourage you to try reading some existing Octave code, especially in the Octave code tree itself, to look for ways to make your code adhere to the Octave coding standards better. Feel free to ping me in a few days if you don't have any changes of your own and haven't heard from me yet.

I would much prefer that you license your functions under the GPLv3 or later, this is the standard Octave license. I would also prefer to see your full name on the Copyright line. and in the mercurial commit log.

Do you know what Matlab does for negative values?

Mike Miller <mtmiller>
Group Member
Mon 12 Mar 2018 01:24:54 PM UTC, original submission:  

Here is an implementation of MATLAB's pow2db function. Any constructive criticism or edit suggestions would be appreciated.

P Sudeepam <sudeepam>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43869:  pow2db.patch added by sudeepam (2KiB - text/x-patch - I believe that I've fixed everything. Please review.)
file #43661:  pow2db.patch added by sudeepam (3KiB - text/x-patch - Fixed negative values MATLAB compatibility, added demos and tests, changed line endings, added appropriate copyright notice.)
file #43523:  pow2db.patch added by sudeepam (938B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by vitalityz (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by sudeepam (Submitted the item)
  • -email is unavailable- added by sudeepam
  •  

    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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-04-10 mtmiller StatusIn Progress Done
        Open/ClosedOpen Closed
    2018-04-10 sudeepam Attached File- Added pow2db.patch, #43869
    2018-03-31 mtmiller StatusNone In Progress
    2018-03-30 mtmiller Assigned toNone mtmiller
        Summary[Octave Forge] (Signal) New function pow2db: convert power value to its decibel value [octave forge] (signal) pow2db: new function
    2018-03-24 sudeepam Attached File- Added pow2db.patch, #43661
    2018-03-12 sudeepam Attached File- Added pow2db.patch, #43523
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code