org-arl / SignalAnalysis.jl

Signal analysis toolbox for Julia
MIT License
41 stars 10 forks source link

Fix padded and partition of signals with custom indices #34

Closed ymtoo closed 1 year ago

ymtoo commented 1 year ago

For padded, before the changes:

julia> x = [1,2,3,4,5];

julia> pad_x = padded(x, (1,1));

julia> pad_pad_x = padded(pad_x, (1,1));

julia> x[1:5]
5-element Vector{Int64}:
 1
 2
 3
 4
 5

julia> pad_x[1:5]
5-element Vector{Int64}:
 1
 2
 3
 4
 5

julia> pad_pad_x[1:5]
5-element Vector{Int64}:
 0
 1
 2
 3
 4

julia> pad_x = padded(signal(randn(10,2), 10), (1,1))
ERROR: MethodError: no method matching padded(::Matrix{Float64}, ::Tuple{Int64, Int64}; delay=0, fill=0.0)
Closest candidates are:
  padded(::MetaArrays.MetaArray{<:Any, SignalAnalysis.SamplingInfo, T}, ::Any; delay, fill) where T at ~/.julia/packages/SignalAnalysis/lEjVN/src/signals.jl:138
  padded(::AbstractVector{T}, ::Any; delay, fill) where T at ~/.julia/packages/SignalAnalysis/lEjVN/src/signals.jl:127
Stacktrace:
 [1] padded(s::MetaArrays.MetaArray{Matrix{Float64}, SignalAnalysis.SamplingInfo, Float64, 2}, padding::Tuple{Int64, Int64}; delay::Int64, fill::Float64)
   @ SignalAnalysis ~/.julia/packages/SignalAnalysis/lEjVN/src/signals.jl:139
 [2] padded(s::MetaArrays.MetaArray{Matrix{Float64}, SignalAnalysis.SamplingInfo, Float64, 2}, padding::Tuple{Int64, Int64})
   @ SignalAnalysis ~/.julia/packages/SignalAnalysis/lEjVN/src/signals.jl:138
 [3] top-level scope
   @ REPL[90]:1

After the changes, padded of a vector or matrix with custom indices works.

julia> pad_pad_x[1:5]
5-element Vector{Int64}:
 1
 2
 3
 4
 5

julia> pad_x = padded(signal(randn(10,2), 10), (1,1))
SampledSignal @ 10.0 Hz, 12×2 PaddedView(0.0, OffsetArray(::Matrix{Float64}, 1:10, 1:2), (0:11, 1:2)) with eltype Float64 with indices 0:11×1:2:
  0.0        0.0
 -0.439627   1.89689
 -0.209597  -0.717943
 -0.328078  -0.37173
 -0.358893  -0.576072
  0.27703    0.0361922
 -0.166934  -0.520583
 -1.91584    0.401654
 -0.334714  -0.201229
 -0.396436   0.181073
 -0.960703   1.54725
  0.0        0.0
ymtoo commented 1 year ago

For partition, before the changes:

julia> pad_x
7-element PaddedView(0, OffsetArray(::Vector{Int64}, 1:5), (0:6,)) with eltype Int64 with indices 0:6:
 0
 1
 2
 3
 4
 5
 0

julia> ps = SignalAnalysis.partition(signal(pad_x, 1), 2);

julia> for p ∈  ps
           println(p)
       end
[1, 2]
[3, 4]
[5, 0]
[0]

After the changes:

julia> for p ∈  ps
           println(p)
       end
[0, 1]
[2, 3]
[4, 5]
[0]
codecov-commenter commented 1 year ago

Codecov Report

Base: 73.44% // Head: 74.44% // Increases project coverage by +1.00% :tada:

Coverage data is based on head (adf9bfa) compared to base (0b9e9ef). Patch coverage: 88.88% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #34 +/- ## ========================================== + Coverage 73.44% 74.44% +1.00% ========================================== Files 10 10 Lines 625 630 +5 ========================================== + Hits 459 469 +10 + Misses 166 161 -5 ``` | [Impacted Files](https://codecov.io/gh/org-arl/SignalAnalysis.jl/pull/34?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=org-arl) | Coverage Δ | | |---|---|---| | [src/signals.jl](https://codecov.io/gh/org-arl/SignalAnalysis.jl/pull/34/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=org-arl#diff-c3JjL3NpZ25hbHMuamw=) | `52.50% <88.88%> (+9.83%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=org-arl). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=org-arl)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

mchitre commented 1 year ago

@ymtoo is this ready to be reviewed and merged?

ymtoo commented 1 year ago

@mchitre yes