Resampling gapless SeisChannel doesn't update `t`

tclements commented 4 years ago

On v1.0.0 resampling a gapless SeisChannel does not update the .t field. This does not affect SeisChannel with gaps or any SeisData.

julia> using SeisIO 
julia> C = SeisIO.RandSeis.randSeisChannel(s=true,c=false)
SeisChannel with 845375 samples
    ID: 4W.JWYRE..ENB
  NAME: 9xqbdIB1VJEhVEGE2n9ULE2P5TYPCEyo
   LOC: [44.2781, -22.3721, 562.839, 38.9882, -33.4714, -65.6544]
    FS: 125.0
  GAIN: 480.947
  RESP: a0 1.0, f0 1.0, 4z, 4p
 UNITS: m/s2
   SRC: randSeisChannel(c=false, nx=0)
  MISC: 11 entries
 NOTES: 1 entries
     T: 2020-06-29T15:19:50 (7 gaps)        
     X: -8.422e-01                          
        -7.590e-01                          
            ...                             
        +4.012e-01                          
        (nx = 845375)     

julia> ungap!(C)
julia> Cnew = resample(C,10.)

# quick tests 
julia> Cnew.fs < C.fs
true

julia> length(Cnew.x) < length(C.x)
true 

julia> Cnew.t == C.t
true # should be false

A quick fix would be adding C.t[2,1] = length(C.x) after https://github.com/jpjones76/SeisIO.jl/blob/99912ee61b2ea0f60db6b9fa9f19c5355bdcd40e/src/Processing/resample.jl#L152-L167

and adding a test with a gapless SeisChannel. Happy to submit a PR on this, if needed.

jpjones76 commented 4 years ago

Adding this to Issue #50 to-do list, which I'm currently working on. I've been investigating resample! behavior in the last few days, and I'm no longer certain that SeisIO should use DSP.jl resample as a model framework. I'm thinking of rewriting it. I'm going to start a new thread about this; any feedback your group can give me (here, or on Skype) would be appreciated.

jpjones76 commented 4 years ago

Fixed on dev, will merge into master if tests pass.

tclements commented 4 years ago

Do you mind pushing a patch release for the resample! update?

jpjones76 commented 4 years ago

Ok! I'm working on this right now. It will happen today (Pacific time).

tclements commented 4 years ago

Awesome, thank you!

jpjones76 commented 4 years ago

Should be done ... with 28 minutes to spare, even. Is it OK to close this issue once the Julia registry updates to the new version?

tclements commented 4 years ago

Yes, for sure - thanks!

jpjones76 / SeisIO.jl

Resampling gapless SeisChannel doesn't update `t` #51