Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update a couple of "extra" EK60 Beam_group1 variables #1099

Merged
merged 6 commits into from
Jul 29, 2023

Conversation

emiliom
Copy link
Collaborator

@emiliom emiliom commented Jul 28, 2023

Follows from #951 (comment), but for EK60. Complements PR #1094.

In Beam_group1:

  • Drop count variable
  • Rename offset to range_sample_offset
  • Cast some variables to integer types: range_sample_offset to dtype=int, transmit_mode to dtype=np.byte (analogous to channel_mode in EK80) and data_type to dtype=np.byte.
  • Improve attributes for range_sample_offset, transmit_mode, and data_type; partly for consistency with EK80

NOTES:

  • The casting to integer types is currently not working! The EchoData DataArrays end up being float types. I'm looking into it.
  • transmit_mode seems to be identical to channel_mode in EK80. @leewujung can you confirm? If they are, let's use the same variable name (channel_mode? Or change EK80 to transmit_mode?) and attribute values.

@emiliom emiliom added the enhancement This makes echopype better label Jul 28, 2023
@emiliom emiliom added this to the 0.8.0 milestone Jul 28, 2023
@emiliom emiliom requested a review from leewujung July 28, 2023 20:20
@emiliom emiliom changed the title Ek60 extra beamgrp vars Update a couple of "extra" EK60 Beam_group1 variables Jul 28, 2023
…tributes. Ensure int casting works as intended via encoding
@emiliom
Copy link
Collaborator Author

emiliom commented Jul 29, 2023

  • Renamed transmit_mode to channel_mode and harmonized its attributes with the attributes from ek80 channel_mode
  • Finalized the specific int types used
  • Ensured that int casting works as intended by adding the following to EXPECTED_VAR_DTYPE in utils/coding.py:
        "channel_mode": np.byte,
        "range_sample_offset": np.int32,

@codecov-commenter
Copy link

codecov-commenter commented Jul 29, 2023

Codecov Report

Merging #1099 (c6c7a5f) into dev (6fc5a1d) will decrease coverage by 3.07%.
Report is 20 commits behind head on dev.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev    #1099      +/-   ##
==========================================
- Coverage   78.12%   75.06%   -3.07%     
==========================================
  Files          65       23      -42     
  Lines        6227     3393    -2834     
==========================================
- Hits         4865     2547    -2318     
+ Misses       1362      846     -516     
Flag Coverage Δ
unittests 75.06% <ø> (-3.07%) ⬇️

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

Files Changed Coverage Δ
echopype/convert/set_groups_ek60.py 92.74% <ø> (ø)
echopype/utils/coding.py 29.88% <ø> (-59.78%) ⬇️

... and 53 files with indirect coverage changes

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

Copy link
Member

@leewujung leewujung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. Thank you!

@emiliom
Copy link
Collaborator Author

emiliom commented Jul 29, 2023

Great, thank you!

@emiliom emiliom merged commit 13be688 into OSOceanAcoustics:dev Jul 29, 2023
3 checks passed
@emiliom emiliom deleted the ek60-extra-beamgrp-vars branch July 29, 2023 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This makes echopype better
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants