search

niosh-mining / obsplus

A Pandas-Centric ObsPy Expansion Pack
GNU Lesser General Public License v3.0
38 stars 8 forks source link

dateline wrap around #240

Closed d-chambers closed 3 years ago

d-chambers commented 3 years ago

Solves the issue of dateline wrap around for get_events and EventBank methods. See #230.

Hey @calum-chamberlain, here is the PR I promised. I didn't mean to steal your thunder, but as you can see, it required a good deal of refactoring some of the more complex parts of ObsPlus so I didn't want to subject you to that. Mind taking a look? A simple manual test on your data would also be good.

calum-chamberlain commented 3 years ago

Happy to have my thunder stolen! I will take a look. Thanks!

codecov-commenter commented 3 years ago

Codecov Report

Merging #240 (05b2163) into master (8483b67) will decrease coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #240      +/-   ##
==========================================
- Coverage   97.89%   97.88%   -0.01%     
==========================================
  Files          39       39              
  Lines        4607     4637      +30     
  Branches      660      663       +3     
==========================================
+ Hits         4510     4539      +29     
  Misses         42       42              
- Partials       55       56       +1
Flag Coverage Δ
unittests 97.88% <100.00%> (-0.01%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
obsplus/bank/core.py 98.08% <ø> (ø)
obsplus/bank/eventbank.py 99.54% <ø> (ø)
obsplus/bank/wavebank.py 97.24% <ø> (ø)
obsplus/datasets/dataset.py 98.26% <ø> (ø)
obsplus/events/merge.py 98.47% <ø> (-0.77%) :arrow_down:
obsplus/events/schema.py 100.00% <ø> (ø)
obsplus/events/validate.py 94.80% <ø> (ø)
obsplus/interfaces.py 100.00% <ø> (ø)
obsplus/stations/get_stations.py 93.84% <ø> (ø)
obsplus/stations/pd.py 94.93% <ø> (ø)
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8483b67...05b2163. Read the comment docs.

calum-chamberlain commented 3 years ago

Looks okay to me, a quick test on my NZ database with:

df = bank.get_event_summary(
    starttime=UTCDateTime(2021, 3, 13), endtime=UTCDateTime(2021, 3, 15), 
    minlongitude=179, maxlongitude=-179)

returns a dataframe with longitudes spanning the 180 degree longitude, :+1:

However, running with:

df = bank.get_event_summary(
    starttime=UTCDateTime(2021, 3, 13), endtime=UTCDateTime(2021, 3, 15), 
    minlongitude=179, maxlongitude=181)

doesn't return over the 180, and doesn't give any warning that 181 appears to be silently fudged to 180? Can this be fixed so that longitude > 180 is mapped nicely?

d-chambers commented 3 years ago

Ok, this PR should solve your problem @calum-chamberlain. If not, we can open another issue/PR.