Added publicID to station #3194

s-schneider · 2022-10-27T13:27:27Z

What does this PR do?

Adds public_id to station objects, if station is read from inventory files from seiscomp.

Why was it initiated? Any relevant Issues?

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).
[ x If the PR is making changes to documentation, docs pages can be built automatically.
Just add the "build_docs" tag to this PR.
Docs will be served at docs.obspy.org/pr/{branch_name} (do not use master branch).
Please post a link to the relevant piece of documentation.
If all tests including network modules (e.g. clients.fdsn) should be tested for the PR,
just add the "test_network" tag to this PR.
All tests still pass.
Any new features or fixed regressions are covered via new tests.
Any new or changed features are fully documented.
Significant changes have been added to CHANGELOG.txt .
First time contributors have added your name to CONTRIBUTORS.txt .
[ x If the changes affect any plotting functions you have checked that the plots
from all the CI builds look correct. Add the "upload_plots" tag so that plotting
outputs are attached as artifacts.
Add the "ready for review" tag when you are ready for the PR to be reviewed.

d-chambers · 2022-11-02T19:41:19Z

Alright @s-schneider, the CI is working again so the failures you see here are probably related to your changes. Take a look and see if you can figure out how to fix them.

megies · 2022-11-03T10:21:58Z

Hmm.. our inventory type classes are kept really close to the StationXML schema. Anything that diverges from it could potentially be trouble in the future, when we might have to adjust to changes in StationXML, or just be confusing to people because there are things that are not in StationXML or are missing when writing to StationXML. So, I'm not sure how I feel about this one.. It's quite a harmless looking change obviously, but it does have the above implications..

megies · 2022-11-03T10:24:31Z

Would it be an option to keep this SCML specific piece of information in the extra part of the station objects maybe?

s-schneider · 2022-11-03T10:36:01Z

Hm if we can add that field to the select method of inventories and networks, that would be a possible solution

megies · 2022-11-03T10:54:41Z

Hm if we can add that field to the select method of inventories and networks, that would be a possible solution

I think I would prefer that, and I don't see a big problem having it in select() if the docstring for it makes clear that it is strictly a SCML thing and wont do anything in most cases.

s-schneider · 2022-11-03T11:09:13Z

Should be no problem, I’ll change that and push an update

s-schneider · 2022-11-22T12:12:03Z

EB_response_NEW_stationXML.zip
I actually just experienced something weird while trying to fix the tests. If I take that example file, read it and write it again, it it adds a description to the response stage input units

  <Stage number="1">
    <PolesZeros name="EBR.2002.091.HE" resourceId="ResponsePAZ#20121207153142.208772.15386">
      <InputUnits>
        <Name>M/S</Name>
        <Description>None</Description>
      </InputUnits>

vs

  <Stage number="1">
    <PolesZeros name="EBR.2002.091.HE" resourceId="ResponsePAZ#20121207153142.208772.15386">
      <InputUnits>
        <Name>M/S</Name>
      </InputUnits>

The code I used:

from obspy import read_inventory

file = "EB_response_NEW_stationXML"
newfile = "EB_response_NEW2_stationXML"
stationxml_inventory = read_inventory(file, format="STATIONXML")
nsmap = {
    'scml': 'seiscompml', 
    'resp': 'http://geofon.gfz-potsdam.de/ns/seiscomp3-schema/0.7'
}
stationxml_inventory.write(newfile, format="STATIONXML", nsmap=nsmap)

megies · 2022-12-01T11:15:49Z

@s-schneider I've rebased this branch on current master and force pushed, to avoid merge conflicts. Can you continue work on this version? Or let me know if that's a problem for you.

megies · 2022-12-01T11:17:47Z

obspy/core/inventory/inventory.py

@@ -639,6 +640,8 @@ def select(self, network=None, station=None, location=None, channel=None,
        :param maxradius: Only include stations/channels within the specified
            maximum number of degrees from the geographic point defined by the
            latitude and longitude parameters.
+        :type public_id: str, optional
+        :param public_id: Only include stations matching with extra.public_id


Please add to the docstring that this mainly is for data read from sc3ml files, so it's clear what this is about here

megies · 2022-12-01T11:18:09Z

obspy/core/inventory/network.py

@@ -457,6 +458,8 @@ def select(self, station=None, location=None, channel=None, time=None,
            will be included in the result. This flag has no effect for
            initially empty stations which will always be retained if they
            are matched by the other parameters.
+        :type public_id: str, optional
+        :param public_id: Only include stations matching with extra.public_id


same as above

megies · 2022-12-01T11:31:24Z

obspy/core/inventory/network.py

+                    else:
+                        continue
+                else:
+                    continue


This confused me at first, although this might be doing what it's supposed to do, I would rewrite this, to make it less nested and less confusing.

Maybe something like..

if public_id is not None: try: sta_public_id = sta.extra.public_id except AttributeError: # deselect stations that do not have this attribute continue else: if not fnmatch.fnmatch(sta_public_id.upper(), public_id.upper()): continue

megies · 2022-12-01T11:33:20Z

obspy/core/tests/test_inventory.py

@@ -408,6 +408,9 @@ def test_inventory_select(self):
        assert sum_stations(inv.select(station="RJOB")) == 9
        assert sum_stations(inv.select(station="R?O*")) == 9

+        # Only Stations with public_id
+        assert sum_stations(inv.select(public_id="TEST")) == 0


There needs to be a test added that is actually filtering out some stations but keeping a desired station. Right now only deselecting everything on arbitrary public_id="..." input is checked.

megies · 2022-12-01T11:33:48Z

obspy/core/tests/test_network.py

@@ -179,6 +179,10 @@ def test_network_select(self):

        # No matching station.
        assert sum(len(i) for i in net.select(station="RR")) == 0
+
+        # Only stations with public_id
+        assert sum(len(i) for i in net.select(public_id="TEST")) == 0


see above, test needs improvement

megies · 2022-12-01T11:37:30Z

obspy/io/seiscomp/inventory.py

@@ -37,7 +38,8 @@

 SOFTWARE_MODULE = "ObsPy %s" % obspy.__version__
 SOFTWARE_URI = "http://www.obspy.org"
-SCHEMA_VERSION = ['0.6', '0.7', '0.8', '0.9', '0.10', '0.11', '0.12']
+SCHEMA_VERSION = "0.11"
+READABLE_VERSIONS = ['0.6', '0.7', '0.8', '0.9', '0.10', '0.11', '0.12']


This change confused me at first, i didn't see any reason for it.. until i looked at individual commits. So this is a relic from some early commits adding WIP on writing sc3ml which later got partially reverted. Ideally these commits should be removed from history, they would just add confusion to the history, I think, considering they never were a working version but just unfinished WIP.

In any case, this change here needs to be reverted too. It's not needed for what this PR is intending.

megies · 2022-12-01T11:37:38Z

obspy/io/seiscomp/inventory.py

@@ -92,7 +94,7 @@ def _read_sc3ml(path_or_file_object, **kwargs):
    root = etree.parse(path_or_file_object).getroot()

    # Code can be used for version 0.6 to 0.12 (Seiscomp 4.x)
-    for version in SCHEMA_VERSION:
+    for version in READABLE_VERSIONS:


megies · 2022-12-01T12:21:23Z

obspy/io/seiscomp/inventory.py

+            'value': public_id,
+            'namespace': ns
+        }
+    )


There are a few things that should be changed here I think.

use namespaces correctly

Namespace should be set to the actual namespace value of the attribute publicID that is being parsed and not quite arbitrarily "seiscmopml". There is an easy way that might be doing the right thing..

namespace = sta_element.nsmap[None]

... but this is working on assumptions how namespaces are set in sc3ml and might break later on.
So to correctly determine the namespace of this attribute, you should check the attribute for a namespace first and if it doesn't have one it should be OK to use the default namespace of it's element

match = re.match(r'{([^}]*)}.*', sta_element.tag) if match: namespace = match.group(1) else: namespace = sta_element.nsmap[None]

I haven't messed around with details of lxml and xml namespaces in a while, but it seems that the default namespace is set on the station element, even though the whole document is operating on the implicit default namespace declared on the root element, so it should be OK to assume we can use that.

Actually, as a last thought.. when accessing public_id = sta_element.get("publicID") that is already assuming that publicID has the default namespace of the station item, I believe, so maybe just using station default namespace is fine..

naming of extra item

I would probably keep the name publicID as the name of the item in extra. Even though that means internally you would look at station.extra.publicID which isn't in line with our Python object naming scheme.. this a) makes it clear that this is not coming from Python land and b) if writing an inventory read from sc3ml as stationxml, your custom attribute would kind of properly show up in the xml file as something like..

<FDSNStationXML xmlns="http://www.fdsn.org/xml/station/1" schemaVersion="1.2"> <Source>scxml import</Source> <Sender>ObsPy Inventory</Sender> <Module/> <ModuleURI/> <Created>2022-12-01T12:14:43.872676Z</Created> <Network code="EB" startDate="1980-01-01T00:00:00.000000Z" restrictedStatus="open"> <Description>SINGLE STATION</Description> <Station xmlns:ns0="seiscompml" code="EBR" startDate=..... ns0:public_id="Station/EB/EBR/2002-04-01T00:00:00.0000Z"> <Latitude unit="DEGREES">40.820599</Latitude> <Longitude unit="DEGREES">0.4933</Longitude> ...

should set type as attribute

Then it would come out as an attribute on StationXML output as opposed to an element.

megies · 2022-12-01T12:22:13Z

obspy/io/seiscomp/tests/test_inventory.py

-        self.sc3ml_inventory.write(sc3ml_bytes, "STATIONXML")
+        self.sc3ml_inventory.write(
+            sc3ml_bytes, "STATIONXML",
+            nsmap={'my_ns': 'seiscompml'}


Should use a proper sc3ml namespace URI

megies · 2022-12-01T12:30:34Z

obspy/core/inventory/network.py

+                    if hasattr(sta.extra, 'publicID'):
+                        if not fnmatch.fnmatch(
+                            sta.extra['publicID'].value.upper(),
+                            public_id.upper()


I'm not sure if publicID in sc3ml is case sensitive, but personally, I'd probably leave the upper() away. I know it's done like this in some other comparisons, and I'm not sure if I agree with that but it is what it is and thats more coming from the SEED perspective where everything was supposed to be all uppercase characters only anyway.

megies · 2022-12-01T12:31:02Z

I pushed some of the adjustments, if you wanna work on the rest?

megies · 2022-12-01T12:32:49Z

CC @filefolder since you worked on sc3ml stuff recently

filefolder · 2022-12-01T21:26:09Z

CC @filefolder since you worked on sc3ml stuff recently

sure, but still out digging holes for another week

megies · 2024-05-22T13:46:54Z

trying to wrap this up for the release @s-schneider @filefolder, maybe you can have a look, I think there was some last things to do

There were some merge conflicts, so need to double-check, most notably 0.13 read support was added in the meantime, so that needs checking if that changes anything

filefolder · 2024-05-24T12:29:18Z

I think the example SCML probably needs to be rinsed through to a newer version, plus you can't specify file storage type (eg Steim2) anymore I think.

While I'm pretty sure reading in event data works fine for 0.13, I never really tested reading inventory files. Currently seeing "0.13 not supported" errors in 1.4.1 but it's a but too late for me to investigate. Attached is an example in the meantime
channel_level.0.13.scml.txt

s-schneider added ready for review PRs that are ready to be reviewed to get marked ready to merge and removed ready for review PRs that are ready to be reviewed to get marked ready to merge labels Oct 27, 2022

s-schneider requested review from megies, trichter and d-chambers as code owners November 22, 2022 08:44

s-schneider removed the ready for review PRs that are ready to be reviewed to get marked ready to merge label Nov 22, 2022

megies force-pushed the sc3_sourceid branch from a177b02 to 110411b Compare December 1, 2022 11:14

megies requested changes Dec 1, 2022

View reviewed changes

megies reviewed Dec 1, 2022

View reviewed changes

megies added this to the 1.5.0 milestone Dec 1, 2022

megies added the .io.seiscomp label Dec 1, 2022

megies requested a review from filefolder December 1, 2022 12:32

s-schneider added 6 commits May 22, 2024 15:38

WIP: write SCML

306abf8

WIP saving publicID

e2ab8cc

added tests and features

90ef333

added changelog

da70454

removed not needed part

2db6252

resolved changelog conflict

db001af

s-schneider and others added 12 commits May 22, 2024 15:44

flake8

0d92227

fixing test

f2005d1

linter

65eb007

linter

de74384

fixing tests

d0193ab

moved public_id to station.extra.public_id

702775d

lintering

9bea09e

fixed network select

c6197ff

fixed extra tagging, ns

611c60a

fixed flake8

bd4b50e

updated writing of scml

f4a2921

sc3ml reading: adjustments to handling "publicID"

1076695

megies force-pushed the sc3_sourceid branch from 6d74af3 to 1076695 Compare May 22, 2024 13:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added publicID to station #3194

Added publicID to station #3194

s-schneider commented Oct 27, 2022 •

edited

d-chambers commented Nov 2, 2022

megies commented Nov 3, 2022

megies commented Nov 3, 2022

s-schneider commented Nov 3, 2022

megies commented Nov 3, 2022 •

edited

s-schneider commented Nov 3, 2022

s-schneider commented Nov 22, 2022

megies commented Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies Dec 1, 2022

megies commented Dec 1, 2022

megies commented Dec 1, 2022

filefolder commented Dec 1, 2022

megies commented May 22, 2024

filefolder commented May 24, 2024

Added publicID to station #3194

Are you sure you want to change the base?

Added publicID to station #3194

Conversation

s-schneider commented Oct 27, 2022 • edited

What does this PR do?

Why was it initiated? Any relevant Issues?

PR Checklist

d-chambers commented Nov 2, 2022

megies commented Nov 3, 2022

megies commented Nov 3, 2022

s-schneider commented Nov 3, 2022

megies commented Nov 3, 2022 • edited

s-schneider commented Nov 3, 2022

s-schneider commented Nov 22, 2022

megies commented Dec 1, 2022

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

use namespaces correctly

naming of extra item

should set type as attribute

Choose a reason for hiding this comment

Choose a reason for hiding this comment

megies commented Dec 1, 2022

megies commented Dec 1, 2022

filefolder commented Dec 1, 2022

megies commented May 22, 2024

filefolder commented May 24, 2024

s-schneider commented Oct 27, 2022 •

edited

megies commented Nov 3, 2022 •

edited