range_samples bug in commongrid.compute_MVBS or metrics\summary_statistics

[slonimer]

The dimensions of an Sv dataset can be seen with "print(ds_Sv.dims)". The dimension "range_sample" disappears and is replaced with "echo_range" after running ep.commongrid.compute_MVBS on AZFP data. This could be fine, but this causes issues later when running metrics\summary_statistics.py functions. The default range_label is "echo_range", but then within the functions, there are hard-coded calls to "range_samples". This bug can be fixed by changing these hard-coded "range_samples" to range_label. OR compute_MVBS should be changed to output both "range_samples" and "echo_range".

[leewujung]

Hey @slonimer - Thanks for reporting this! The metrics package is quite outdated as we haven't used or maintained it for long time.

The compute_MVBS function would only have echo_range as a coordinate because the MVBS computation is done along a physical unit (m). The range_sample is just the sequential index of data samples.

Do you think you could help fixing this bug by changing the metrics package functions to accept a range_var input argument? I recommend this because it is the same argument name used in compute_MVBS, so echo_range would also be the default and the argument would also accepts depth:

echopype/echopype/commongrid/api.py

Lines 31 to 43 in 9bd6b13

	def compute_MVBS(
	ds_Sv: xr.Dataset,
	range_var: Literal["echo_range", "depth"] = "echo_range",
	range_bin: str = "20m",
	ping_time_bin: str = "20s",
	method: str = "map-reduce",
	reindex: bool = False,
	skipna: bool = True,
	fill_value: float = np.nan,
	closed: Literal["left", "right"] = "left",
	range_var_max: str = None,
	**flox_kwargs,
	):