mspass-team / mspass

Massive Parallel Analysis System for Seismologists
https://mspass.org
BSD 3-Clause "New" or "Revised" License
28 stars 11 forks source link

Fix splicing #437

Closed pavlis closed 1 year ago

pavlis commented 1 year ago

This is an important bug fix. The previous version of the C++ function splice_segments had a subtle, undetected bug that this branch fixes. One of the dangers in multiple inheritance in OOP is method names can overlap. In this case, the bug was caused by using the wrong "size" method reference for TimeSeries objects used in the algorithm.

This also fixes two typos that caused cause the python merge function to abort in most situations.

This particular bug fix is a case-in-point of why MsPASS needs a lot more exercising with real data with real data problems. I found this while working on real data downloaded from IRIS with obspy.

@wangyinz if this passes tests as it should you should merge this asap. This is a major bug that makes an important low-level function invalid.

pavlis commented 1 year ago

@wangyinz unexpected error emerged from pytest run. I hadn't expected that and failed to run a local pytest. I'll deal with that directly and hopefully we can then merge this branch.

codecov[bot] commented 1 year ago

Codecov Report

Merging #437 (07c8586) into master (902eb56) will increase coverage by 0.02%. The diff coverage is 82.60%.

@@            Coverage Diff             @@
##           master     #437      +/-   ##
==========================================
+ Coverage   54.59%   54.61%   +0.02%     
==========================================
  Files         141      141              
  Lines       21560    21574      +14     
==========================================
+ Hits        11770    11782      +12     
- Misses       9790     9792       +2     
Impacted Files Coverage Δ
python/mspasspy/algorithms/window.py 56.42% <33.33%> (ø)
cxx/src/lib/algorithms/splicing.cc 51.20% <50.00%> (-0.42%) :arrow_down:
python/mspasspy/util/converter.py 84.70% <100.00%> (+0.09%) :arrow_up:
python/mspasspy/util/decorators.py 65.20% <100.00%> (+1.04%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

wangyinz commented 1 year ago

The error in the test was due to the deletion of the data copy line in the timeseries_copy_helper, which makes the copied data empty. I got that fixed and had this branch merged.