Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate welch_pgram without explicitly given window #581

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

martinholters
Copy link
Member

In preparation of #152/#153 to be done in v0.9.

@martinholters martinholters added this to the 0.8 milestone Nov 7, 2024
Copy link

codecov bot commented Nov 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.84%. Comparing base (3c1b783) to head (eeb56d5).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #581   +/-   ##
=======================================
  Coverage   97.84%   97.84%           
=======================================
  Files          19       19           
  Lines        3252     3255    +3     
=======================================
+ Hits         3182     3185    +3     
  Misses         70       70           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +610 to 620
julia> power(welch_pgram(x; window=nothing)) # 1-sided periodogram
2-element Vector{Float64}:
0.9523809523809523
0.04761904761904761

julia> power(welch_pgram(x; onesided=false)) # 2-sided periodogram
julia> power(welch_pgram(x; onesided=false, window=nothing)) # 2-sided periodogram
3-element Vector{Float64}:
0.9523809523809523
0.023809523809523805
0.023809523809523805

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the short length of x, the default n==3 and

julia> hanning(3)
3-element Vector{Float64}:
 0.0
 1.0
 0.0

is not at all better than the default rect window. In the next example, where n=5 is explicitly set, I've changed the window to hanning as it somewhat reasonable there. I think from a pedagogical viewpoint this is ok, and I'd like to keep a complete rewrite of the example out of the scope of this PR.

Sidenote: a variant of the windows without the zeros at the edges might a useful here, e.g.

julia> hanning(5)[begin+1:end-1]
3-element Vector{Float64}:
 0.5
 1.0
 0.5

But that's also beyond the scope of this PR.

@martinholters
Copy link
Member Author

CC @JakeZw

@martinholters martinholters force-pushed the mh/deprecated-no-window-welch-pgram branch from af61887 to eeb56d5 Compare November 11, 2024 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant