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

chore: climate typing #53

Merged
merged 1 commit into from
Jun 1, 2024
Merged

Conversation

chemelli74
Copy link
Collaborator

SSIA

@chemelli74 chemelli74 merged commit faf9b6c into wuwentao:master Jun 1, 2024
4 checks passed
@chemelli74 chemelli74 deleted the chemelli74-climate branch June 1, 2024 14:08
]

def __init__(self, device, entity_key, zone):
def __init__(self, device: Any, entity_key: str, zone: str) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, zone is 0 or 1, not str

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the doubt but then looking at the rest of the code I went for "str". Let's create a new PR to change it

def current_temperature(self):
return None
def current_temperature(self) -> float:
return 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning 0 is incorrect, return type should be -> float | None

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want None, then we need to change the superclass as well ;-)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

caibinqing added a commit to caibinqing/midea_ac_lan that referenced this pull request Jun 2, 2024
chemelli74 pushed a commit that referenced this pull request Jun 2, 2024
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.

3 participants