Rename `range` to `echo_range` and `range_bin` to `range_sample`

[leewujung]

Right now we have a data variable range that contains the range (in meters) of the calibrated Sv samples. However, range is part of the reserved vocabulary so we should avoid that.

What should we rename it to? sonar_range?

If I understand correctly, in the IMOS convention, they use depth for this (so it is the sonar range + the transducer_depth/water_level or range_offset, see #516). But that assumes that the sonar is pointed vertically and corrected with any offset. IMO we should aim for a more general case here.

This will be a breaking change, so we should get this in in v0.6.0.

Tagging @b-reyes @emiliom

[gavinmacaulay]

echo_range?

[emiliom]

To add a wrinkle ... When we rename range, say to echo_range, should we also rename range_bin to echo_range_bin just for clarity??

[b-reyes]

From what I understand, this range value is created in compute_Sv. On the other hand, range_bin is a dimension that is continually used throughout the EchoData object. For consistency, I would say that we keep the name range_bin, unless we want to change the name of the dimension range_bin.

[leewujung]

To add yet another wrinkle, should range_bin just be sample_number? 🤯

[b-reyes]

Yes, from how you @leewujung explained it to me yesterday, it seems like sample_number may be a better name for range_bin.

[emiliom]

So, I guess my vote is for echo_range.

As for sample_number, one thing I don't like is that it's divorced from any obvious linkage to the along-range coordinate. BUT, if it's widely used in active acoustics and sonar lingo, then that's fine with me.

[leewujung]

Maybe the most explicit is sample_number_along_range, which is how I would say it in explaining things. It is just a bit long...

Another choice is sample_index_along_range, which is probably more correct, since "number" could be referring to the total number of samples, but "index" seems would naturally just be the index within an array?

[emiliom]

I figured there were no good options ... Yeah, those two are very long, specially for coordinates. Sigh.

[leewujung]

sample_index ....

[leewujung]

Ok, how about echo_range and range_sample (with a description that this is the index number) or range_sample_index

[emiliom]

range_sample is nice and compact. I can't say if the absence of a number or index suffix makes it very ambiguous by itself. @b-reyes what do you think?

The need for good attributes goes w/o saying!

[b-reyes]

It is unfortunate that range_sample_index is a little too long, I like that name. However, for compactness, my vote is for range_sample too. With the caveat that we add the attribute that this is "the sample index along the range". From the discussion so far, it also looks like the vote is to turn range into echo_range.

[leewujung]

Ok, let's go with these then: range --> echo_range, range_bin --> range_sample

Looking at this pair, it's not too bad! 😄

[b-reyes]

Sounds good! I will start working on this change.

[b-reyes]

As proposed by @leewujung and @lsetiawan in our meeting today, it will be important to add a check and depreciation warning to open_converted. This will be triggered anytime range_bin occurs in the converted files (or a certain echopype version is identified). Under the covers, we can change range_bin to range_sample and urge the users to resave the new EchoData object that we create. From my understanding, the change from range to echo_range will not require a similar check (please correct me, if I am wrong).

[emiliom]

Can this issue be closed now? The only thing I see that's not addressed yet is this:

As proposed by @leewujung and @lsetiawan in our meeting today, it will be important to add a check and depreciation warning to open_converted. This will be triggered anytime range_bin occurs in the converted files (or a certain echopype version is identified). Under the covers, we can change range_bin to range_sample and urge the users to resave the new EchoData object that we create. From my understanding, the change from range to echo_range will not require a similar check (please correct me, if I am wrong).

But I think that issue is broader than the original scope of this issue, since it involves the whole backward compatibility topic. See #596.

[b-reyes]

I think we should maybe leave this open until we create an issue that specifically outlines what needs to be done for the backward compatibility. #596 is only modifying how we store different versions of the structure, not how we should be converting the actual EchoData object between versions.

[emiliom]

Agreed. Sounds good

[leewujung]

I can volunteer to put together that issue and you all can chime in there to make the task list complete. We can use that issue to track #596, this one, and other related ones. This https://github.com/OSOceanAcoustics/echopype/issues/596#issuecomment-1079279255 gets partially there but it'd be good to have a clearer task by task list that is not buried in another issue.

[emiliom]

Thanks @leewujung ! That sounds good. I think that new issue should fold in the relevant parts of this one, and we should then close this one rather than track it. The core elements of this issue are already addressed.

[leewujung]

I started on this in #606 but I think we need to make a decision there -- please chime in!

[leewujung]

I think it is now safe to close this.