OSOceanAcoustics / echopype

Enabling interoperability and scalability in ocean sonar data analysis
https://echopype.readthedocs.io/
Apache License 2.0
89 stars 70 forks source link

Added alpha-version code for new AZFP6 format #1323

Open dash-uvic opened 1 month ago

dash-uvic commented 1 month ago

Added in initial code for the new AZFP6 sensor format for *.azfp files from ASL Environmental services. The data is in a new format and has additional optional GPS information. The XML file is now embedded into the data file. No tests added.

leewujung commented 1 month ago

Hey @dash-uvic : Thanks for the contribution! Could you fix the pre-commit errors? Also let us know what files you would like to add to the CI. As always, small files are better!

dash-uvic commented 3 weeks ago

Fixed pre-commit errors.

codecov-commenter commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 17.30280% with 325 lines in your changes missing coverage. Please review.

Project coverage is 44.76%. Comparing base (9f56124) to head (96e52fa). Report is 54 commits behind head on main.

Files Patch % Lines
echopype/convert/parse_azfp6.py 16.14% 239 Missing :warning:
echopype/convert/set_groups_azfp6.py 16.66% 85 Missing :warning:
echopype/core.py 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1323 +/- ## =========================================== - Coverage 83.52% 44.76% -38.76% =========================================== Files 64 66 +2 Lines 5686 6194 +508 =========================================== - Hits 4749 2773 -1976 - Misses 937 3421 +2484 ``` | [Flag](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1323/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/OSOceanAcoustics/echopype/pull/1323/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics) | `44.76% <17.30%> (-38.76%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=OSOceanAcoustics#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

leewujung commented 3 weeks ago

Hey @dash-uvic : Thanks for fixing the pre-commits.

I've just approved for the CI to run on this PR. If you could add file conversion tests to run on some example test files, that will allow us to review the changes more easily.

The way we organized test data now is for you to share the files with us with the specific path you would like those to be in under echopype/test_data/ (so these should correspond with what your tests use). We will add them to a google drive that the CI pulls automatically on each test run. This will change hopefully later this summer/fall to use GitHub Release Assets, but for now we will continue with this setup.

leewujung commented 5 days ago

@dash-uvic : I saw that you added some tests and just "approved" the workflow to run - I think usually for a PR it just needs to be approved once and then it'll always run for new changes. If you see that stalled somehow feel free to ping me directly.

dash-uvic commented 2 days ago

@leewujung What's the best method for adding/sending you the test files (average ~50M but there is a larger ~400M file)?

leewujung commented 2 days ago

@dash-uvic : Is there a reason why the files need to be that large? If not, could you generated some smaller files? Large files will make CI runs a lot slower.

dash-uvic commented 2 days ago

@leewujung Yeah I can probably truncate the larger azfp file down to fewer timesteps and remove unrequired fields from the expected output Mat file. I think doing that I can get them down to ~10M if that's sufficient.

leewujung commented 2 days ago

@dash-uvic : Yes ~10MB would be nice. Thanks!

Also if it is easy for you to put the files on a cloud folder somewhere, I can download them and put them in the docker image we currently use for the CI. We would like to move all files to use the github release assets, but don't have time to do it just yet...

dash-uvic commented 15 hours ago

@leewujung Ok sounds goods, I'll put them on a cloud and email you the link next week.