-
Notifications
You must be signed in to change notification settings - Fork 109
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
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. |
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 | ||
|
There was a problem hiding this comment.
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.
CC @JakeZw |
af61887
to
eeb56d5
Compare
In preparation of #152/#153 to be done in v0.9.