OSOceanAcoustics / echopype

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

Removed the unused `ALL` param in parser object [all tests ci] #1203

Closed praneethratna closed 11 months ago

praneethratna commented 11 months ago

Addresses #857 by removing unused "ALL" param in parser object.

CC @leewujung

codecov-commenter commented 11 months ago

Codecov Report

Merging #1203 (af57d39) into dev (9dfe5ba) will increase coverage by 0.24%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #1203      +/-   ##
==========================================
+ Coverage   77.08%   77.32%   +0.24%     
==========================================
  Files          67       67              
  Lines        5921     5879      -42     
==========================================
- Hits         4564     4546      -18     
+ Misses       1357     1333      -24     
Flag Coverage Δ
unittests 77.32% <100.00%> (+0.24%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
echopype/convert/api.py 85.81% <100.00%> (-0.20%) :arrow_down:
echopype/convert/parse_base.py 92.40% <100.00%> (+4.54%) :arrow_up:
echopype/convert/parse_ek60.py 100.00% <ø> (+30.00%) :arrow_up:
echopype/convert/parse_ek80.py 100.00% <ø> (+50.00%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

praneethratna commented 11 months ago

Closing and re-opening the PR to rerun all the tests.

leewujung commented 11 months ago

@praneethratna : I just have one minor comment above.

Also: #1185 is just merged! The merge conflicts are because of the removal of everything related to dgram_zarr_vars. I took a quick look and thought it should be straightforward to resolve the conflicts, but please let me know if you run into cases you are unsure about. Once the merge is done I think we can merge this PR. Thank you!!

praneethratna commented 11 months ago

@praneethratna : I just have one minor comment above.

Also: #1185 is just merged! The merge conflicts are because of the removal of everything related to dgram_zarr_vars. I took a quick look and thought it should be straightforward to resolve the conflicts, but please let me know if you run into cases you are unsure about. Once the merge is done I think we can merge this PR. Thank you!!

@leewujung I have resolved all the merge conflicts and rebased the code with latest changes from dev. I think we can merge this PR once the tests pass! Thank you!

praneethratna commented 11 months ago

@leewujung I messed up this PR while resolving the merge conflicts, so I've made a new pull request #1214 with all the changes and all the tests passing, please do check that! I'm closing this PR. Thanks!