BinaryPayloadBuilder.to_string type hint

[jamesbraza]

pymodbus==3.2.2's BinaryPayloadBuilder.to_string actually returns bytes.

Two questions:

1. Should we type hint this with bytes?
2. Should it be named encode or to_bytes, since it doesn't return a string?
   • Note: BinaryPayloadBuilder.__str__ already exists, so I would think to_string is redundant

[janiversen]

type hinting with bytes is a good idea.

renaming to to_bytes is a good idea (remember to to add it to API_changes.rst)

str is used to e.g. print the object, but should not replace a to_* method.

[janiversen]

if your preference is encode() it's ok with me.

[alexrudd2]

I agree the type hint should be bytes, but do not have a preference on to_string versus encode.

Just for information, if you set the following in setup.cfg

[mypy]
check_untyped_defs = True

then you can use mypy reveal_type.

[jamesbraza]

Okay the PR is now merged! One comment, is there's also __bytes__.

I more leave this as food for thought... it might be slick to use __bytes__ instead of encode:

assert builder.encode() == bytes(builder)

[janiversen]

no it would not, the '__' methods are magic methods describing the object or adding a specific quality.

If an app wants to manipulate the object buffer it should do so with a clear method call.

But that of course is just my opinion.