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

Changed Stream.get_gaps() to identify gaps within masked Traces #2300

Merged
merged 6 commits into from Feb 10, 2019

Conversation

Projects
None yet
3 participants
@sboltz
Copy link
Contributor

sboltz commented Jan 31, 2019

What does this PR do?

This PR adds logic to Stream.get_gaps() to identify gaps in Streams that have Traces with masked arrays (previously merged Streams).

Why was it initiated? Any relevant Issues?

This PR was initiated to address issue #2299.

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
  • If the PR is making changes to documentation, docs pages can be built automatically.
    Just remove the space in the following string after the + sign: "+DOCS"
  • If any network modules should be tested for the PR, add them as a comma separated list
    (e.g. clients.fdsn,clients.arclink) after the colon in the following magic string: "+TESTS:"
    (you can also add "ALL" to just simply run all tests across all modules)
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests.
  • Any new or changed features have are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
  • First time contributors have added your name to CONTRIBUTORS.txt .
@d-chambers

This comment has been minimized.

Copy link
Member

d-chambers commented Jan 31, 2019

Awesome @sboltz 🚀

The output of circleci indicates line 199 of stream.py needs to be shortened by one character:

obspy/core/stream.py:799:80: E501 line too long (80 > 79 characters)

I am not 100% sure what is going on with conda in the travis builds but it looks like the --no-update-dependencies flag here needs to be changed to -no-update-deps. This might have been changed in Anaconda 4.6 but I didn't see mention of it in the changelog. You might just give it a try and see if the travis builds work.

@sboltz

This comment has been minimized.

Copy link
Contributor Author

sboltz commented Jan 31, 2019

All right, we'll see if this works.

@megies megies added this to the 1.2.0 milestone Jan 31, 2019

@@ -21,6 +21,7 @@ master:
resource_ids which are not found via the normal means (see #2279).
* Calling Stream.write(...) on an empty stream will now raise an
ObsPyException consistently across all I/O plugins (see #2201)
* Stream.get_gaps() will now check for gaps within previously merged Traces.

This comment has been minimized.

@megies

megies Jan 31, 2019

Member

I think this should be rephrased a bit to make it clear this is about masked-array Traces, i.e. merged with Stream.merge(0), because obviously get_gaps() is not able to know about gaps when gaps are filled with whatever values.

This comment has been minimized.

@sboltz

sboltz Feb 6, 2019

Author Contributor

That makes sense. I will make the changes you suggest here and in the next comment.

@@ -758,7 +758,7 @@ def get_gaps(self, min_gap=None, max_gap=None):
Please be aware that no sorting and checking of stations, channels, ...
is done. This method only compares the start and end times of the
Traces.
Traces and gaps within Traces.

This comment has been minimized.

@megies

megies Jan 31, 2019

Member

Maybe also rephrase a little bit here

@megies

megies approved these changes Jan 31, 2019

Copy link
Member

megies left a comment

Just nitpicks about changelog/docs wordings, otherwise looks good! Thanks @sboltz 🚀

@megies megies added the .core label Jan 31, 2019

sboltz and others added some commits Feb 10, 2019

@megies

megies approved these changes Feb 10, 2019

Copy link
Member

megies left a comment

Looks good to me, merging. Thanks a bunch @sboltz 🎉

@megies megies merged commit eb0b1c2 into obspy:master Feb 10, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment