Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Use monkeytype to apply typehints #44

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hmaerki
Copy link

@hmaerki hmaerki commented Jan 23, 2025

Adding typehints

I saw this issue #30 and tried to add type hints.

This is how far I got - now it is time to get feedback.

What I did:

pytest tests --monkeytype-output=monkeytype.sqlite3
monkeytype run example_mbus.py

This way I collected runtime data from the test folder and from example_mbus.py running agains real meters.

Then I applied the typehints monkeytype apply meterbus.zzz.

The result is here WIP: Applies types hints using monkeytype.

The easy typehints

About 60% of the typehints are simple and make sense.

The complex typehints

Then I continued manually starting to resolve the Any type hints.
For example:
def _parse_vifx(self) -> Any:
which I manually resolved to:
def _parse_vifx(self) -> Tuple[Union[None, int, float], Union[None, str, MeasureUnit], Union[None, str, VIFUnit, VIFUnitExt, VIFUnitSecExt], Union[None, VIFUnitEnhExt]]:

Another complex structure is:
Before:

lut = {
        # E000 0nnn    Energy Wh (0.001Wh to 10000Wh)
        0x00: (1.0e-3, MeasureUnit.WH, VIFUnit.ENERGY_WH),

and with the typehints resolved:

lut: Dict[int, Tuple[float, Union[str, MeasureUnit], Union[str, VIFUnit, VIFUnitExt, VIFUnitSecExt]]] = {
        # E000 0nnn    Energy Wh (0.001Wh to 10000Wh)
        0x00: (1.0e-3, MeasureUnit.WH, VIFUnit.ENERGY_WH),

These changes are here: hmaerki@d6984ab

How to continue?

  • First of all: Why bother? The code works also without type hints and there are many tests.
  • The benefit of adding typehints is
    • Internally: That it will be easier to maintain and understand the code.
    • Externally: The library is easier to use.

If typehints are really wanted, I see several work packages:

  • A) Add the easy typehints (60% of what monkeytype found).
  • B) Add typehints specially for the API (meterbus/init.py, meterbus/serial.py)
  • C) Get rid of decimal.Decimal in favor of float.
  • D) Rething the types in core_objects. There are many enumerations and there datatypes spread in the whole package. There might be ways to hide all the different datatypes.
  • E) def _parse_vifx() returns a tuple of four values (Tuple[Union[None, int, float], Union[None, str, MeasureUnit], Union[None, str, VIFUnit, VIFUnitExt, VIFUnitSecExt], Union[None, VIFUnitEnhExt]]:). There might be a cleaner way to do that.

My proposal would be to go for A) and B).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant