Standardize use of sonar metadata as Sonar global attributes vs (for EK80 only) variables
#992
Conversation
…on; add long_name to other sonar metadata variables, and rename one such var
echopype/convert/set_groups_ek80.py
Outdated
| "transducer_name": (["channel"], var["transducer_name"]), | ||
| "sonar_serial_number": (["channel"], var["channel_id_short"]), | ||
| "sonar_software_name": ( | ||
| "sonar_serial_number": ( | ||
| ["channel"], | ||
| var["application_name"], | ||
| ), # identical for all channels | ||
| "sonar_software_version": ( | ||
| var["channel_id_short"], | ||
| { | ||
| "long_name": "Sonar serial number", | ||
| }, | ||
| ), | ||
| "transducer_serial_number": ( | ||
| ["channel"], | ||
| var["serial_number"], | ||
| { | ||
| "long_name": "Transducer serial number", | ||
| }, | ||
| ), | ||
| "transducer_name": ( | ||
| ["channel"], | ||
| var["application_version"], | ||
| ), # identical for all channels | ||
| var["transducer_name"], | ||
| { | ||
| "long_name": "Transducer name", | ||
| }, | ||
| ), |
There was a problem hiding this comment.
I think something is not quite right with these changes.
channel_id_shortare the transducer model plus serial numbertransducer_namecontains the transducer model.serial_numbercontains the transceiver serial number (transceivers are things like GPT and WBT).
See #212 (comment) for example printouts, I think you could see how these numbers are combined to become what's in channel_id
There was a problem hiding this comment.
To clarify, my changes in the block you're commenting on amount to just two things:
- Rename
serial_numbertotransducer_serial_number - Add a
long_nameattribute with values that are just the variable name spelled out.
I'm not sure what changes you're proposing. But since transducer_name and serial_number are variable names from echopype, not the convention, we can rename them as we see fit. Are you proposing we rename them to transducer_model and transceiver_serial_number, respectively?
As for sonar_serial_number, that's defined as a global attribute in the convention and as a variable by echopype, for EK80 only b/c it varies with channel. In your examples in #212, it's "0" in one case (which I interpret as not available / known) and "202" in another (a suspiciously short value for a serial number, IMHO). Should we use that parameter instead of channel_id_short? In #212 (comment) you had equated transducer serial number with channel_id_short.
There was a problem hiding this comment.
My comment above is to clarify what each of the variables are coming out from the parser, and that transceiver and transducer are different things.
I think sonar_serial_number is ill-defined since it can mean too many things.
Below is what I propose:
- store the parsed
transducer_nameastransducer_namein echopype (ie. no change), so that it is easy for folks familiar with EK80 raw data know what they are - store the parsed
serial_numberastransceiver_serial_numberin echopype, so that it is specific that it is not the transducer serial number - store the parsed
transducer_serial_numberastransducer_serial_numberin echopype (ie. no change), for clarify - do not store
sonar_serial_numberfor EK80: This is marked as recommended and not mandatory in the convention. I think it is confusing what it is in this context. And, not populating it makes it consistent with EK60. 😬
channel_id_short seems a combination of transducer_name and transducer_serial_number, so I am thinking that we can safely ignore it.
There was a problem hiding this comment.
I've pushed a commit that implements this:
sonar_serial_numberis an empty global attribute, no longer a variable- variables:
transducer_name, based on parsertransducer_nameparametertransducer_serial_number, based on parsertransducer_serial_numberparametertransceiver_serial_number, based on parserserial_numberparameter
That follows what you recommended, but please review.
…mber will be an empty attribute; clarify transceiver_serial_number vs transdicer_serial_number variables
|
Looks great, please feel free to merge. Thanks for putting in comments about the |
|
Thanks! Will merge. |
Addresses all tasks in #681 involving sonar metadata in the
Sonargroup as implemented in echopype vs in the SONAR-netCDF convention.Nearly all the changes are to EK80, where the main issue was the presence of some sonar metadata as variables rather than global attributes. Specifically:
sonar_software_nameandsonar_software_versionto be global attributes rather than variablesserial_numbertotransducer_serial_number(@leewujung please confirm if this is accurate; see Varying variables inSonargroup amongst sensors #681 (comment))long_nameattribute to variablessonar_serial_number,transducer_serial_numberandtransducer_nameFor AZFP, Set
sonar_software_nameto "AZFP" instead of "Based on AZFP Matlab Toolbox".