Feature/new windows#658
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #658 +/- ##
==========================================
+ Coverage 98.01% 98.03% +0.01%
==========================================
Files 19 19
Lines 3279 3303 +24
==========================================
+ Hits 3214 3238 +24
Misses 65 65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Done |
wheeheee
left a comment
wheeheee
left a comment
There was a problem hiding this comment.
Looks fine. Maybe the other maintainers have further comments, so I'll just leave this lying around for 2 weeks or so.
wheeheee
left a comment
wheeheee
left a comment
There was a problem hiding this comment.
Some small typos.
I'm not sure if it's in there somewhere, but how did you generate the reference data? It would be good to put that in comments near where they are loaded.
|
I'm not 100% sure about the codecov complaint, as the mentioned lines have been there before. Is this due to some coverage checker update? |
|
Yeah, seems to be. It's the same on master, after bumping setup-julia from v2 to v3. You can rebase onto master and add on from there, hopefully it all goes away. |
As described in (On the use of windows for harmonic analysis with the discrete Fourier transform)[https://ieeexplore.ieee.org/document/1455106], this commit adds the derived minimum sidelobe versions of the 4-term and 3-term blackman harris windows. To not clutter the window function names, the arbitration is done by an additional named parameter (term), that decides for the used term number. To not change to much of the DSP implementation for windows, only the 4-term version is included in the automatic 2D-expansion of the windows, since this is the one that is available in MATLAB as well.
To preserve the conventional kwargs structure of all windows, the `term` argument for blackmanharris and nuttall are change into normal positional arguments. This implies some updates for the tests and the make_matrix function, but it works quite well. Now it is even possible to create a two-dimensional nuttall window, where the first dimension consists of a 4-term nuttall window of arbitrary length while the second dimension consists of a 3-term nuttall window of a different length.
daniel-s-w
force-pushed
the
feature/new_windows
branch
from
36e7536 to
ea694ce
Compare
3 participants
As suggested in #657 this PR adds two conventional windows to DSP (
blackmanharrisandflattop), so they do not need manual user implementation withmakewindow. The flattop as well as the Blackman-Harris 4-term window coefficients are the same as the ones in MATLAB. Additionally, theblackmanharrisfunction supports the selection of the 3-term minimum sidelobe version (not given in MATLAB).The
gen_winplots.jlhas been updated such that the negative values for the flattop window are shown.closes #657