Conversation
|
I have not figured out how to properly set the cell size and blanking distance for high resolution (HR) profiles, so tbd on that. |
|
Hi @jmcvey3 , thanks a lot for the great work. You are a legend! I will pull and test it hopefully next week. |
akeeste
left a comment
There was a problem hiding this comment.
Thanks @jmcvey3. This generally looks good to me. Do we need a new test to cover Aquadopp data?
@MRCGit-collab @MogoBRE if you can test this PR with your datasets that would be very helpful!
There was a problem hiding this comment.
These files are now ignored by .gitignore. I suggest we remove this file entirely
There was a problem hiding this comment.
Hold that thought - they're needed in test_data, but not the next directory up, where they're generated and not deleted if that test fails.
|
Tagging #356 for some of the original discussion |
Yes I need to write tests |
| units="1", | ||
| ), | ||
| "PressureMSB": _VarAtts( | ||
| "pressure": _VarAtts( |
There was a problem hiding this comment.
@jmcvey3, is it possible that changing these variable names would break any analysis code downstream for MHKiT users? IDK if it is worth having some deprecation strategy here or what makes sense. This could quickly get complex and difficult, but curious how you handle changes here more than anything.
| default_val=nan, | ||
| units="1", | ||
| long_name="Acoustic Signal Amplitude", | ||
| standard_name="signal_intensity_from_multibeam_acoustic_doppler_velocity_sensor_in_sea_water", |
There was a problem hiding this comment.
@jmcvey3 I think that adding standard names from the CF standard names table is a nice addition here. Do you think including the description would also be valuable?
Also, one nit here that I don't know how to solve is the sea_water part of the cf convention names. I think there are likely situations where the instrument is deployed in NOT sea water and that standard name may be incorrect and misleading. I don't have an obvious solution, just more of an observation here. I don't other non sea water variants signal_intensity_* in the cf standard name table.
PR to add functionality to read Aquadopp data packets, but this code has only been tested on Aquadopp profilers (not single-point Aquadopps). Note that Aquadopp/AWAC waves functionality has also been added but is untested.
I've done a decent bit of cleanup of the nortek.py code in this PR as well.
The PR also simplifies the code related to the non-cabled ADV's "orientation_down" parameter. (This parameter exists because the z-axis is flipped on the non-cabled version of the ADV). This PR no longer reads the bit set in each burst ping, which is wont to oscillate for an unknown reason, but instead takes the value from the datafile's header. This means that DOLfYN assumes that a non-cabled ADV is not moved after it starts running.
Cabled ADVs, AWACs and Aquadopps also have the "orientation_down" parameter for user information, but it is not needed otherwise.