zserio icon indicating copy to clipboard operation
zserio copied to clipboard

Python: Consider not to use underscores in parameters for fields

Open mikir opened this issue 4 years ago • 3 comments

Python extension now accepts in the constructor the field values. Considering the following schema

struct SimpleStructure
{
    bit:3       numberA;
    uint8       numberB;
    bit:7       numberC;
};

the Python extension will generate the following constructor:

    def __init__(
            self,
            numberA_: int = int(),
            numberB_: int = int(),
            numberC_: int = int()) -> None:
        self._numberA_ = numberA_
        self._numberB_ = numberB_
        self._numberC_ = numberC_

which could be called using keyword arguments:

simpleStructure = SimpleStructure(numberA_=2, numberB_=4, numberC_=6)

Underscores in keyword arguments for fields are not intuitive and do not look good. These underscores are easy solution of the clash with the parameter self. Example:

struct StructureWithClash
{
    bit:3 self;
};

If this easy solution becomes annoying, we can implement checking of such clashes and remove underscores in parameters for fields.

mikir avatar Feb 02 '21 11:02 mikir

Investigate if self can be renamed. See https://stackoverflow.com/questions/4131582/why-is-self-only-a-convention-and-not-a-real-python-keyword.

mikir avatar Feb 11 '21 14:02 mikir

self can be freely renamed however pylint will start to complain:

E0213: Method should have "self" as first argument (no-self-argument)

It's a question if it's better to rename self or to rename all field parameters. Currently, the best idea is to check if there is a clash and if so all field parameters will be appended by underscore.

mikir avatar Feb 12 '21 14:02 mikir

The same problem is with argument cls.

If field parameters will be appended by underscore only if there is a clash, the generated API would become incompatible if new field self or cls will be added/remove. Such behavior could be unexpected by the user.

If we rename self and cls arguments, the generated code will look strange and the readability will suffer.

Because we can't see any good generic solution for this and because underscores do not seem as a big pain for the users, we will leave it as it is for now.

mikir avatar Feb 19 '21 10:02 mikir