Skip to content

Conversation

@Lash-L
Copy link
Collaborator

@Lash-L Lash-L commented Apr 11, 2023

Trying to get mypy fully functioning so it can be a step in our ci pipeline.

mypy checks and makes sure we are not making typing errors

@Lash-L
Copy link
Collaborator Author

Lash-L commented Apr 11, 2023

@humbertogontijo

a few things:

  1. Perhaps we turn on squish commits for PRs? That way we just have one note in the changelog rather than multiple? Not sure

  2. After this I plan to also incorporate styling so that we don't end up having weird issues with our styling being different and unneeded changes?

  3. The left mypy errors are stumping me a bit, any ideas? (ignore the ones from cli, I'm not sure why that isn't being excluded)

A whole lot of :

Invalid index type "str" for "RoborockEnum"; expected type "Union[int, slice]"

Perhaps we switch to using a backpropegated version of StrEnum from python 3.11?

Then

Variable "roborock.code_mappings.RoborockDockTypeCode" is not valid as a type

finally, something with the superclasses for roborockmqttclient are wrong, it works runtime, but gives warnigns in ide:

roborock/cloud_api.py:123: error: "RoborockMqttClient" has no attribute "_keepalive"; maybe "_check_keepalive"?

roborock/cloud_api.py:128: error: "_check_keepalive" undefined in superclass

@humbertogontijo
Copy link
Collaborator

  1. I left only squash and merge options for PRs
  2. Styling would be nice. Since I use PyCharm and you VSCode, could you look for one that is compatible for both?
    3.1. I don't know the impact of using python 3.11 for the homeassistant integration
    3.2. MqttClient needs to override paho mqtt private method _check_keepalive because of some weird staling that happens after connected to the mqtt for some time (I dont think we can get rid of this one)

@Lash-L
Copy link
Collaborator Author

Lash-L commented Apr 12, 2023

  1. Styling would be nice. Since I use PyCharm and you VSCode, could you look for one that is compatible for both?

Yeah - I'll probably look at some of the HA ones

3.1. I don't know the impact of using python 3.11 for the homeassistant integration

I meant using our current version, but just importing StrEnum from a backpropagated package - adds another dependency but may have better luck?

3.2. MqttClient needs to override paho mqtt private method _check_keepalive because of some weird staling that happens after connected to the mqtt for some time (I dont think we can get rid of this one)

Any idea why it doesn't think check_keepalive or even just _keepalive are valid in the superclass? I can see them, but something about the inheritance is weird

@Lash-L
Copy link
Collaborator Author

Lash-L commented Apr 13, 2023

@humbertogontijo This should be good to go as long as you don't have any reviews. I was able to fix some instances of the 'code' is not a type, but just setting the type hint to roborock enum and then check the name. But I couldn't fix the ones in the dataclasses without rewriting parts of the as dict or from dict functions. So I just ignored them for now.

Regardless, this gives us a rather good foundation.

I have tested it locally with no problems.

@Lash-L Lash-L marked this pull request as ready for review April 13, 2023 00:21
@humbertogontijo humbertogontijo merged commit 16bd2d1 into Python-roborock:main Apr 13, 2023
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.

2 participants