Add read support for HypoDD pha files #2378

trichter · 2019-04-11T14:25:41Z

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
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 .

trichter · 2019-04-11T14:38:56Z

+DOCS

krischer

Thanks for this! Some comments in-line.

krischer · 2019-04-11T14:57:34Z

obspy/io/hypodd/pha.py

+            temp = f.readline()
+    except Exception:
+        return False
+    return temp.startswith(b'#') and len(temp.split()) == 15


That seems like a pretty weak test to me and we have a couple of other text/csv like formats and the danger of false positives in the wild is IMHO pretty big. How about also checking if the first couple of entries also make up a valid date time object?

We should also consider putting weak detection routines at the very end of the autodetection chain.

obspy/io/hypodd/pha.py

krischer · 2019-04-11T15:09:17Z

obspy/io/hypodd/pha.py

+        for net in inventory:
+            for sta in net:
+                if len(sta) == 0:
+                    temp = id_map.get(sta.code, id_default)


I'm a bit confused here - if the station is already in the id_map the users have given some mapping explicitly (they might want to for example map the station to a different network code). I'd rather just continue here as user specified things should not be overwritten. But I might also just be missing something here.

I think you are right. I will change this. I wanted to also support inventories with only network and station information, but that does not make much sense.

I think I will also add a warning when the construction of the id_map would not be unique (e.g. BH and HH channels in inventory) and move these functions to some util module, because they are also used by evt plugin. Is it OK if I add a new file obspy/io/util.py?

obspy/io/hypodd/pha.py

obspy/io/hypodd/tests/test_pha.py

krischer · 2019-04-11T15:15:52Z

misc/docs/source/packages/index.rst

@@ -95,6 +95,7 @@ categories.*
    obspy.io.css
    obspy.io.gcf
    obspy.io.gse2
+    obspy.io.hypodd


That's the waveform plugin section. Please add it to the event plugin section.

obspy/io/hypodd/pha.py

Co-Authored-By: trichter <tom.eulenfeld@uni-jena.de>

megies · 2019-04-12T08:13:12Z

CircleCI: obspy/io/hypodd/pha.py:61:27: E999 SyntaxError: invalid syntax

trichter · 2019-04-12T08:57:17Z

CircleCI: obspy/io/hypodd/pha.py:61:27: E999 SyntaxError: invalid syntax

Yeah, I already saw this. Committing inside the browser is a bit error prone.

trichter · 2019-04-12T09:45:08Z

@krischer thanks for the review. I think the PR is ready.

obspy/core/util/misc.py

megies · 2019-04-12T12:08:17Z

obspy/io/hypodd/__init__.py

+>>> print(cat)
+2 Event(s) in Catalog:
+2025-05-14T14:35:35.510000Z | +40.225,  +10.450 | 3.5 None
+2025-05-14T15:43:05.280000Z | +40.223,  +10.450 | 1.8 None


And here they say you can't predict earthquakes! 😎

setup.py

megies

Looks pretty much ready to go

krischer · 2019-04-12T13:44:41Z

obspy/core/util/__init__.py

@@ -34,7 +34,8 @@
                                  PROJ4_VERSION)
 from obspy.core.util.misc import (BAND_CODE, CatchOutput, complexify_string,
                                  guess_delta, loadtxt, score_at_percentile,
-                                  to_int_or_zero, SuppressOutput)
+                                  to_int_or_zero, SuppressOutput,
+                                  _seed_id_map)


Any reason to put this here? It could just be directly imported from misc.

Hmh, it has a leading underscore, so its private, but I thought it might even be of use outside obspy. And even inside obspy its easier to import from core.util instead of core.util.misc. What do you think?

Our current policy is that everything with a leading underscore is internal only and we are free to change the API of those functions and even remove them so I'd advise against using it outside of ObsPy.

And these "import aliases" just make it harder to find where things originate from and the convenience improvement from not having to write .misc is IMHO not worth the trade-off. But that is just my opinion and feel free to choose.

Our current policy is that everything with a leading underscore is internal only and we are free to change the API of those functions and even remove them so I'd advise against using it outside of ObsPy.

On the other hand, if you do want to use it, you can rename it and remove the leading underscore. But then we need proper tests for it as well (I didnt check if there already are tests for this, maybe there are already).

krischer

Only a minor nitpick from me this time around. Feel free to merge once you think its ready!

trichter · 2019-04-12T18:40:18Z

OK, I addressed it. However, I am not authorized to merge.

krischer · 2019-04-12T20:28:34Z

OK, I addressed it. However, I am not authorized to merge.

Yea this is not good. I opened #2384 to discuss it.

megies · 2019-04-15T09:32:56Z

🎉 thanks for the PR @trichter

trichter added 5 commits April 11, 2019 16:18

add read support for HypoDD pha files

49442c1

add plugin in setup

ce3d9e8

changelog

220fef2

add doc page

1b8a731

pep8

fef50f3

add PR number

de95bed

krischer requested changes Apr 11, 2019

View reviewed changes

krischer and others added 8 commits April 11, 2019 23:22

pha: change call to UTCDateTime

f0bc398

Co-Authored-By: trichter <tom.eulenfeld@uni-jena.de>

pha: add id for magnitude

936f214

Co-Authored-By: trichter <tom.eulenfeld@uni-jena.de>

pha: add id for origin

67c546d

Co-Authored-By: trichter <tom.eulenfeld@uni-jena.de>

pha: add prefix to event id

52c037c

Co-Authored-By: trichter <tom.eulenfeld@uni-jena.de>

pha: move doc link to event section

6a079a6

pha: fix SyntaxError

cffe8b8

oha: make _is_pha unambiguous

270eb11

refactor _seed_id_map and do not overwrite user-supplied id_map

44d5523

trichter added 2 commits April 12, 2019 10:40

pha: enhance tests

d892f9c

generalize _seed_id_map and move to obspy.core.util.misc

0b12ddc

reuse _seed_id_map in evt plugin

3d38523

megies reviewed Apr 12, 2019

View reviewed changes

obspy/core/util/misc.py Outdated Show resolved Hide resolved

megies reviewed Apr 12, 2019

View reviewed changes

setup.py Outdated Show resolved Hide resolved

megies added this to the 1.2.0 milestone Apr 12, 2019

megies added .io issues generally related to our read/write plugins enhancement feature request labels Apr 12, 2019

megies approved these changes Apr 12, 2019

View reviewed changes

doc string starts with newline

38d7740

trichter added 2 commits April 12, 2019 15:10

rename format to HYPODDPHA

6eaadb3

add negative test for _is_pha

b1a4122

krischer reviewed Apr 12, 2019

View reviewed changes

krischer approved these changes Apr 12, 2019

View reviewed changes

trichter added 3 commits April 12, 2019 20:08

evt: import _seed_id_map from misc

a88730e

pha: import _seed_id_map from misc

3f1f24c

remove _seed_id_map from core.util

a992b7b

krischer merged commit e97fe91 into master Apr 12, 2019

krischer deleted the hypodd branch April 12, 2019 20:28

megies added this to Done in Release 1.2.0 May 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add read support for HypoDD pha files #2378

Add read support for HypoDD pha files #2378

trichter commented Apr 11, 2019

trichter commented Apr 11, 2019

krischer left a comment

krischer Apr 11, 2019

megies Apr 12, 2019

krischer Apr 11, 2019

trichter Apr 11, 2019

krischer Apr 11, 2019

megies commented Apr 12, 2019

trichter commented Apr 12, 2019

trichter commented Apr 12, 2019 •

edited

megies Apr 12, 2019

megies left a comment

krischer Apr 12, 2019

trichter Apr 12, 2019

krischer Apr 12, 2019

megies Apr 15, 2019

krischer left a comment

trichter commented Apr 12, 2019

krischer commented Apr 12, 2019

megies commented Apr 15, 2019

Add read support for HypoDD pha files #2378

Add read support for HypoDD pha files #2378

Conversation

trichter commented Apr 11, 2019

PR Checklist

trichter commented Apr 11, 2019

krischer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

megies commented Apr 12, 2019

trichter commented Apr 12, 2019

trichter commented Apr 12, 2019 • edited

Choose a reason for hiding this comment

megies left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krischer left a comment

Choose a reason for hiding this comment

trichter commented Apr 12, 2019

krischer commented Apr 12, 2019

megies commented Apr 15, 2019

trichter commented Apr 12, 2019 •

edited