PIC-IRIS / PH5

Library of PH5 clients, apis, and utilities
Other
15 stars 9 forks source link

Additional check for ph5_validate, remove location warnings for ph5tostationxml #435

Closed damhuonglan closed 3 years ago

damhuonglan commented 4 years ago

What does this PR do?

Solve #410: All the warnings for station's location includes:

will be excluded for ph5tostationxml.

Solve #433:

Checklist

damhuonglan commented 3 years ago

This is related to issue #443

433

timronan commented 3 years ago

After reviewing this issue it seems like we should flag missing units r location/X[Y,Z]/units_s = '' or location/X[Y,Z]/units_s = in both the event and array tables as errors.

The fields "location/coordinate_system_s, projection_s, projection_s , ellipsoid_s" should remain as trigger warnings. Noven seems to be the issue in not populating these fields. These fields could still be manually populated and segdtoph5 populates these fields.

timronan commented 3 years ago

What does everyone think about my last comment? How do we feel about this issue?

hrotman-pic commented 3 years ago

For location/Z/unit_s=unknown, that hasn't caused a problem yet with creating the stationXML. Should we assume that will continue to be OK?

timronan commented 3 years ago

@hrotman-pic how are location/Z/unit_s fields populated with the value "unknown". Could this be fixed and have a more descriptive answer injected?

hrotman-pic commented 3 years ago

@timronan This is a bug that happens with Fairfield nodes & segdtoph5. In the meeting it sounded like Alissa was planning to open an issue to request a fix so it is correctly populated as location/Z/unit_s=m, but I am thinking of the Fairfield PH5s already in the archive: will it cause a metadata extraction problem later, even though it doesn't seem to now? I wonder if we should flag this case as a warning so we can catch those archives.

hrotman-pic commented 3 years ago

Apologies for commenting on a merged PR.

@timronan should the last commit you added catch a case with location/Z lines in the array table: location/Z/value_d=1.0 location/Z/units_s=unknown and issue a warning for each channel with location/Z/units_s=unknown, in ph5_validate.log? I am on our server that auto-updates so I think I'm on the most up to date commit. ph5_validate didn't issue a warning. (If this isn't the right place to comment, please let me know.)

timronan commented 3 years ago

@hrotman-pic good catch. That last commit only looks for location/Z/units_s=unknown in the event table, and not the array table. We can start another PR go catch this warning in the array table as well.