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

 
 

patch #9587: [octave forge] (signal) db2pow: new function

Submitter:  P Sudeepam <sudeepam>
Submitted:  Sun 11 Mar 2018 03:02:13 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:42 PM UTC, comment #8: 

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
Tue 10 Apr 2018 04:35:23 PM UTC, comment #7: 

I'm sorry about the whitespaces. I did know about that style but it seems I ignored them by accident. I'll keep all these things in mind in future submissions. Thank you for the reviews.

P Sudeepam <sudeepam>
Tue 10 Apr 2018 04:13:03 PM UTC, comment #6: 

I am posting comments here for both db2pow and pow2db.

In your latest patches you are still missing some important whitespace. Specifically:

  • write "10 .^ (db/10)" instead of "10.^(db/10)", as I mentioned in comment #4
  • write "db2pow (x)", "pow2db (x)", and "error (x)" instead of "db2pow(x)", "pow2db(x)", and "error(x)"


The doc strings could use some spell checking and are missing some punctuation.

No need to make yet another revision, I will take these patches, clean them up as mentioned above and add them to the signal package.

Mike Miller <mtmiller>
Group Member
Sat 31 Mar 2018 04:04:29 PM UTC, comment #5: 

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 05:54:55 AM UTC, comment #4: 

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 db2pow verbose" to see the test specifics).

Please indent line 40 with the rest of the function body. And please use spaces around all binary arithmetic operators, like you did in pow2db.m.

Have you tried the doctest function yet? Can you install doctest (pkg install -forge doctest) and run "doctest db2pow" 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
Tue 13 Mar 2018 12:41:30 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?

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

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.

Mike Miller <mtmiller>
Group Member
Mon 12 Mar 2018 01:21:35 PM UTC, comment #1: 

Here is an implementation of MATLAB's db2pow function. I was not very sure about certain things such as writing the copyright comments since this is my first patch. Any constructive criticism and edit suggestions would be appreciated.

P Sudeepam <sudeepam>
Sun 11 Mar 2018 03:02:13 PM UTC, original submission:  


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 #43870:  db2pow.patch added by sudeepam (2KiB - text/x-patch - I believe that I've fixed everything. Please review.)
file #43660:  db2pow.patch added by sudeepam (520B - text/x-patch - Fixed a minor typo)
file #43658:  db2pow.patch added by sudeepam (2KiB - text/x-patch - Added tests, demos. Changed to unix line endings. Changed to preffered license.)
file #43514:  db2pow.patch added by sudeepam (906B - 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 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 10 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 db2pow.patch, #43870
    2018-03-31 mtmiller StatusNone In Progress
    2018-03-30 mtmiller Assigned toNone mtmiller
        Summary[Octave Forge] (Signal) New function: db2pow convert db value to its power value [octave forge] (signal) db2pow: new function
    2018-03-24 sudeepam Attached File- Added db2pow.patch, #43660
    2018-03-24 sudeepam Attached File- Added db2pow.patch, #43658
    2018-03-11 sudeepam Attached File- Added db2pow.patch, #43514
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code