Removing pointless (?) setter/getter patter from some algorithms

[1ucian0]

Some of the code imported from Aqua came with this pattern of pointless-looking setter/getters. I might be missing something here. Is it okey to remove them?

[manoelmarques]

One thing I might say is that, specially in the result classes, you lose the property type information and sphinx documentation allowed by typehints for each property getter/setter removed.

[Cryoris]

I agree with Manoel, there's value in having the documentation in some cases. However I think that we indeed to have a lot of boilerplate here.

One solution would be to put the documentation in the class docstring. To keep the type info (which we should) can we use Python's dataclass? https://docs.python.org/3/library/dataclasses.html

[1ucian0]

yeap... I think class docstring is the way to go.

[woodsp-ibm]

Its a public interface - the variables and there interaction with the class may be self-evident but it should have documentation as needed. Instance vars can be documented via Sphinx - they are not for now from what I have seen - only class vars.

The getter/setter, even when its a simple private field, is consistent with what is done elsewhere in Qiskit. This change, if its wanted, should be done in a consistent way throughout - maybe doing it on a couple of algos in a PR here was to open up some discussion with more concrete examples. Maybe it should be opened up as an Issue to bring the subject up more generally with examples to have a wider discussion in Qiskit.

[levbishop]

To keep the type info (which we should) can we use Python's dataclass? https://docs.python.org/3/library/dataclasses.html

Dataclass is really nice but it's a python 3.7+ feature and currently we still support 3.6 (see #5301)

[mtreinish]

For documentation, Sphinx will autogenerate the attribute entries for @property methods and use the method docstring for that getter method as the attribute docstring. However, there is no requirement to do that just to get documentation, the easiest way is to manually list the attribute in the docstring for the class. With the napolean plugin we use for docstrings it would be something like:

class Foo:
    """Class for something
    
    Attributes:
        my_special_attr (int): This attribute has the number of something for the class
   """

That being said I'm not sure what this PR is trying to solve tbh, there is nothing wrong with using a setter/getter combo like this, and it's not like it exposes us to more bugs, and it kind of makes the api explicit. Like saying we expect these fields to explicitly be retrieved and set via the attribute access.