Additional test and related fixes stemming from Platform group PR #1058 [all tests ci] #1061
Conversation
…ng empty lat & lon variables
…l_offset and the removal of channel dim from ek60 platform vars
Codecov Report
@@ Coverage Diff @@
## dev #1061 +/- ##
==========================================
- Coverage 78.19% 77.62% -0.58%
==========================================
Files 67 67
Lines 6192 6202 +10
==========================================
- Hits 4842 4814 -28
- Misses 1350 1388 +38
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
emiliom
changed the title
There was a problem hiding this comment.
@emiliom : thanks, the changes look good. I am still puzzled by the segmentation problem, but it can be investigated later as long as we don't normally run into that problem. It is also weird to me that I ran into that in the Sonar/Beam_group1 rather than the Vendor_specific group like you had.
For the sake of clarity, could you add the definition of water_level and vertical_offset with a simple diagram of which one means which for an underwater vehicle and a ship? This should actually be in the docs, but if we have something parked here it'll be easy to plug them in afterwards.
Other than that, please feel free to merge!
|
Great, thanks! Merging. I created a new issue, #1062, about the documentation to be added (great idea, BTW). |
|
This PR is related to #1051 |
…ceanAcoustics#1058 [all tests ci] (OSOceanAcoustics#1061) * Fix processing level L1A test to account for AZFP open_raw now creating empty lat & lon variables * Remove obsolete environmental parameters tests * Fix test_plot tests and visualize api involving water_level > vertical_offset and the removal of channel dim from ek60 platform vars * Cosmetic change simply to trigger all ci tests * Remove remnant test case for ek60 vertical_offset channel dim; correct data-type typing hint
Adapting what I wrote in #1058 (comment). The previously missed test failures were:
tests/calibrate/test_calibrate.py::test_env_params: I removed the test. It was testing the obsoleteEnvParamclass incalibrate/env_params_old.py, which was replaced, though not 1:1, byenv_params.py::ENVPARAMand a new accompanying test. Per @leewujung :env_params.pydoes not have the interpolation capability but it is more complete in terms of being able to take user input dictionary tocompute_Sv. @leewujung will follow up with a PR to removeenv_params_old.pytests/utils/test_processinglevels_integration.py::test_raw_to_mvbs[AZFP-AZFP-raw_and_xml_paths1-extras1]: Upgraded it to account for the case (especially with AZFP) where Platformlatitudeandlongitudevariables are present but are all nan. That was not the case for AZFP prior to PR Bring more consistency in the Platform group across sensors on conversion #1058tests/visualize/test_plot.py::test_water_level_echodata[True-False]: Made substantial changes to the test and tovisualize/api.pymodule to change the use ofwater_leveltovertical_offset(and similar references to water level), and to remove in the test the handling of the former, inappropriatechanneldimension in ek60water_levelandvertical_offset. Made a few minor, additional improvements along the way, such as for type hints.Note: I successfully ran the complete test suite locally except I temporarily removed the "netcdf4" export engine option from
test_convert_source_target_locs.py::export_engine, to sidestep the new, unexpected (and unrelated) problem with theVendor_specificgroup and netcdf4 files. On the first run of the CI, all tests ran successfully and the netcdf4 segmentation fault did not take place. Hmm.