Skip to content
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

Properly reading MiniSEED files with microsecond wrap arounds #1531

Merged
merged 4 commits into from Sep 21, 2016

Conversation

krischer
Copy link
Member

This PR enables ObsPy to read MiniSEED files with microseconds wrap arounds.

Times in SEED have a .0001 second field which according to the spec must be in 0-9999. I've encountered a file that has a value of 10000.

ObsPy now interprets these as a additional seconds. libmseed internally appears to do the same so I guess these files were encountered before.

@Jollyfant
Copy link
Contributor

Looks good. I can pass review if tests complete. How about raising a warning when microseconds > 10000. It might be indicative of two fishy bytes instead of silently wrapping around to seconds.

@Jollyfant
Copy link
Contributor

Not because I feel very strongly about this but because reviewing makes me feel like I need to contribute.

Raises it both - in the Python and on the C side.
@krischer
Copy link
Member Author

krischer commented Sep 20, 2016

Not because I feel very strongly about this but because reviewing makes me feel like I need to contribute.

Oh please do :-) We really need more people to review changes in any case. And in general people who are familiar with large pieces of the code base.

How about raising a warning when microseconds > 10000. It might be indicative of two fishy bytes instead of silently wrapping around to seconds.

Good idea and done in the latest commit. It now looks like this:

In [2]: obspy.read("./tests/data/microsecond_wrap.mseed")
/Users/lion/workspace/code/obspy/obspy/io/mseed/util.py:432: UserWarning: Record contains a fractional seconds (.0001 secs) of 10000 - the maximum strictly allowed value is 9999. It will be interpreted as one or more additional seconds.
  category=UserWarning)
/Users/lion/workspace/code/obspy/obspy/io/mseed/core.py:423: InternalMSEEDReadingWarning: readMSEEDBuffer(): Record with offset=0 has a fractional second (.0001 seconds) of 10000. This is not strictly valid but will be interpreted as one or more additional seconds.
  warnings.warn(*_i)
Out[2]:
1 Trace(s) in Stream:
IM.NV32..BHE | 2008-01-08T04:58:06.000000Z - 2008-01-08T04:58:12.900000Z | 40.0 Hz, 277 samples

It's a bit verbose as its raised twice. The first one is raised once per file - its from the Python part that reads the very first record to set-up some stuff. The second one is in the C part and will be raised for every single record that has this.

I think this is okay as it does violate the standard. If people are annoyed by this they can always just catch the warnings. What do you think?

@Jollyfant
Copy link
Contributor

Ok nice. I think it's better to raise the warning and risk annoying someone than to ignore it. It should only print the warning twice if the first record has the problem, which would be very rarely.

@krischer
Copy link
Member Author

Now passes CI - ready for review!

Copy link
Contributor

@Jollyfant Jollyfant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comprehensive change and descriptive tests. Ready to be merged.

@krischer krischer merged commit cb32961 into obspy:maintenance_1.0.x Sep 21, 2016
@QuLogic QuLogic added this to the 1.0.3 milestone Sep 21, 2016
@megies megies deleted the mseed-microsecond-wrap branch September 26, 2016 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants