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

Actual changes in echopype #971 #31

Open
wants to merge 48 commits into
base: azfp-range
Choose a base branch
from
Open

Conversation

leewujung
Copy link
Owner

No description provided.

Copy link

@lsetiawan lsetiawan left a comment

Choose a reason for hiding this comment

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

Looks good overall. Again, minor changes to be made. 👍🏽

@leewujung
Copy link
Owner Author

Thanks @lsetiawan ! I think I've addressed/committed all of your comments.

@leewujung
Copy link
Owner Author

Oops, some tests have failed. Let me look into that.

@leewujung
Copy link
Owner Author

Ok, fixed!

…to `max_mb` (OSOceanAcoustics#962)

* Update `offload_to_zarr` to `use_swap` as well as `max_zarr_mb` to `max_mb`

* Require raw_file and sonar_model

* Fix linting error

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Copy link

@lsetiawan lsetiawan left a comment

Choose a reason for hiding this comment

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

LGTM! Very minor docstring stuff, which you can do whatever you'd like 😄

emiliom and others added 17 commits March 11, 2023 08:27
* Update CITATION.cff

* Add Brandon to software citation in CITATION.cff

* docs: add citation section
…SOceanAcoustics#954)

* Expand on validate_output_path docstring

* Update docstring to vary examples

* Remove plural 's' in combined_echodatas.zarr

* Update combined_echodatas to combined_echodata

* Update temp_output paths

* Change run-test to use temp_ouput

* Fix circular imports

* Remove unused pathlib in core

* Update text to `combine_echodata`

Co-authored-by: Wu-Jung Lee <[email protected]>

* Update api docstring

Co-authored-by: Wu-Jung Lee <[email protected]>

* Update docs/source/convert.rst

Co-authored-by: Wu-Jung Lee <[email protected]>

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Wu-Jung Lee <[email protected]>
Co-authored-by: Don Setiawan <[email protected]>
* add some initial sketch on the changes

* draft up cal_params.sanitize_user_cal_dict and param_dict2da

* draft up get_cal_params_EK_new and _test_interp_da to handle all triage and interpolating of cal params

* add test_get_interp_da and placeholder for other cal_param tests

* add pre-commit fixes

* add test for sanitize_user_cal_dict

* tidy up comments

* revise/rename param2da for organizing individual param, add test

* add param2da use to sanitize_user_cal_dict, add corresponding tests

* fix logic in get_cal_params_EK_new and clearly lay out alternative data type in _get_interp_da

* revise default cal/env_params handling in CalibrateBase.__init__, still need to test these

* change CalibrateEK80._cal_complex_samples to use params from self.cal_params; still need to do the same for CalibrateEK60 and CalibrateAZFP

* add tests for only subset of channel needs to be interpolated

* fix a few more things in _get_intep_da for EK80 CW mode data; note down new tests needed

* minor tweak to use get_cal_params_EK_new for calibrating EK60

* remove the old get_cal_params_EK

* completely remove old private functions in CalibrateEK80, run pre-commit

* revise get_cal_params_AZFP to use sanitize_user_cal_dict

* squeeze and drop beam dimension earlier in CalibrateAZFP._cal_power_samples

* rename get_cal_params_EK_new to get_cal_params_EK

* add test_get_cal_params_AZFP

* add da name to individual param data in sanitize_user_cal_dict

* allow coord combination (cal_channel_id, cal_frequency) in sanitize_user_cal_dict

* add interp for user-input freq-dependent param values, add tests

* add test_get_cal_params_EK60, change sanitize_user_cal_dict handling of EK types

* move sequence of functions in cal_params.py

* add test_get_vend_cal_params_power

* add cal param intake integration test for EK80 BB complex

* add cal param intake integration test for EK80 CW complex

* add test_cal_params_intake_EK60

* fix bug re looping through params in PARAM_BEAM_NAME_MAP

* add test_cal_params_intake_AZFP

* fix bug in AZFP env param intake

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove else in param2da for readability

* improve sonar_type typing

* improve param2da p_val docstring

* improve sanitize_user_cal_dict docstring

* remove redundant else in sanitize_user_cal_dict

* improve _get_interp_da docstring

* change .data to .values

* use self.waveform_mode in CalibrateEK60.__init__

* fix get_cal_params_AZFP docstring

* improve get_cal_params_EK docstring

* change default_dict to default_params, add Literal

* use f-strings instead of % syntax in get_vend_filter_EK80

* add sonar_type as attribute for Calibrate* classes

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Don Setiawan <[email protected]>
…oustics#968)

* reorder echo_range dimensions at azfp cal output

* add test_range_dimensions for AZFP

* add test_range_dimensions for EK60

* add test_range_dimensions for EK80 BB complex and CW complex

* add test_range_dimensions for EK80 CW power

* Define common dimension order global variable. Fix spelling error in test_range_integration.py

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Emilio Mayorga <[email protected]>
…e, keep beam dim removal in get_angle_power_sample
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants