Modify `Beam_groupX` descriptions

[b-reyes]

After looking at the html representation of the EchoData object for the EK80 sensor, I noticed that there seems to be a printed sentence that does not quite make sense. The sentence is:

"Only exists if complex backscatter data they already in Sonar/Beam_group1"

This is printed out because the current repr uses the Beam_groupX descriptions in 1.0.yml. I think this sentence should be modified.

Additionally, should we be using 1.0.yml to create the descriptions for these beam groups? I think it would be better to use the self._beamgroups variable we create in set_groups (if possible) e.g.:

https://github.com/OSOceanAcoustics/echopype/blob/5033dc7e683416a731bbf1722ee14e96726f8558/echopype/convert/set_groups_ek80.py#L17-L29

@emiliom @lsetiawan what do you think?

[lsetiawan]

So many of these descriptions everywhere! I didn't notice that the sentence was funky until now. Thanks for pointing that out @b-reyes. I definitely agree that we should be using the descriptions within the set groups instead of 1.0.yml since that is what is being used in set groups. Maybe there should be a consolidated place for these?

[leewujung]

I definitely agree that we should be using the descriptions within the set groups instead of 1.0.yml since that is what is being used in set groups. Maybe there should be a consolidated place for these?

Can the set group methods pull from 1.0.yml? Or if things in 1.0.yml has to be completely the same as defined in the convention, there could probably be something similar to utils/prov.py for these descriptions.

[b-reyes]

I definitely agree that we should be using the descriptions within the set groups instead of 1.0.yml since that is what is being used in set groups. Maybe there should be a consolidated place for these?

Can the set group methods pull from 1.0.yml? Or if things in 1.0.yml has to be completely the same as defined in the convention, there could probably be something similar to utils/prov.py for these descriptions.

From my understanding the self._beamgroups can change from sensor to sensor. If this is the case, then I think pulling from 1.0.yml might not be the best idea. I think it might be better to have a consolidated place for these type of descriptions.

[leewujung]

From my understanding the self._beamgroups can change from sensor to sensor. If this is the case, then I think pulling from 1.0.yml might not be the best idea. I think it might be better to have a consolidated place for these type of descriptions.

Oh that's true, good call!

[emiliom]

From my understanding the self._beamgroups can change from sensor to sensor. If this is the case, then I think pulling from 1.0.yml might not be the best idea. I think it might be better to have a consolidated place for these type of descriptions.

Exactly. I added this _beamgroups property to address the decision we made to have sensor-customized beam group descriptions. The intent is that this property should be used directly wherever it's needed. I do think this sits well in the sensor module, set_groups_ek80.py, because that's where sensor specifics are defined. But in order to access this dictionary information more conveniently, the dict that's assigned to self._beamgroups could be pulled out and defined instead as a global constant in the module, which then would be used in the __init__ method.

1.0.yml can't accommodate beam group descriptions that vary with the sensor.

[b-reyes]

Exactly. I added this _beamgroups property to address the decision we made to have sensor-customized beam group descriptions. The intent is that this property should be used directly wherever it's needed. I do think this sits well in the sensor module, set_groups_ek80.py, because that's where sensor specifics are defined. But in order to access this dictionary information more conveniently, the dict that's assigned to self._beamgroups could be pulled out and defined instead as a global constant in the module, which then would be used in the init method.

I am trying to picture what you are recommending. Is it something like this?

class SetGroupsEK60(SetGroupsBase):
    beam_groups = [{"name": "Beam_group1", "descr": "blah blah"}]
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self._beamgroups = beam_groups 

[emiliom]

Almost:

beam_groups = [{"name": "Beam_group1", "descr": "blah blah"}]

class SetGroupsEK60(SetGroupsBase):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self._beamgroups = beam_groups 

Or come to think of it, maybe no change is needed? The echodata repr operates on an echodata object, so the _beamgroups property is easily accessible to the repr function. I was -- wrongly -- thinking that an import might be needed.

[b-reyes]

Or come to think of it, maybe no change is needed? The echodata repr operates on an echodata object, so the _beamgroups property is easily accessible to the repr function. I was -- wrongly -- thinking that an import might be needed.

I looked into it and we should be able to access the information via self['Sonar'].beam_group_descr in the EchoData class. Then we can pass this to the repr.

[leewujung]

repr: https://github.com/OSOceanAcoustics/echopype/blob/7231b59111966de6b3595f0f07597262dc72a87f/echopype/echodata/widgets/utils.py#L26

[b-reyes]

As shown in the comment of issue 671, @emiliom and I have come to the conclusion that we will get the beam_group descriptions from ed['Sonar'].beam_group_descr when constructing the repr.

[b-reyes]

PR #880 addresses this issue, closing.