Skip to content

Conversation

@alexrudd2
Copy link
Collaborator

Builds on #1388

See #1360

@janiversen
Copy link
Collaborator

Looks good. Hope the most parts can be done without tooo much editing.

@alexrudd2
Copy link
Collaborator Author

Looks good. Hope the most parts can be done without tooo much editing.

Enabling all the rules gives ~1000 errors, mostly disallow_untyped_defs. I think we should more cleanly define internal versus external methods, since external ones are more important to be typed correctly.

@janiversen
Copy link
Collaborator

That is a plan I highly agree with. As I work through the code, all real externals will be or are defined in init as external symbols, that can at least to identify the external interfaces.

Can e.g. "disallow_untyped_defs" be restricted to a module ? if so, we can consider making a API module, and have all external classes (pure interface classes there). That does not sound difficult to do, and will just affect the import statement in the apps.

@janiversen
Copy link
Collaborator

Once you update your branch (also in the other PRs) you will possibly see more problems.

@alexrudd2
Copy link
Collaborator Author

Can e.g. "disallow_untyped_defs" be restricted to a module ? if so, we can consider making a API module, and have all external classes (pure interface classes there). That does not sound difficult to do, and will just affect the import statement in the apps.

I believe so, yes.

@janiversen
Copy link
Collaborator

Then I suggest (but you decide):

  • activate the options that are (relative) easy to solve
  • keep the rest deactivated
  • Group the ones you think we should activate for in a interface module
  • Group the rest as unwanted
    I have done a similar thing in the pylint setup, and it helps me remember that an option was tried and discarded etc.

You do not need to do it all in this PR, just put it up for review when you want.

@janiversen
Copy link
Collaborator

and you have a couple of conflicts to solve...

@alexrudd2
Copy link
Collaborator Author

and you have a couple of conflicts to solve...

Yes, I am rebasing now locally

@alexrudd2 alexrudd2 marked this pull request as ready for review February 27, 2023 16:31
@alexrudd2 alexrudd2 requested a review from janiversen February 27, 2023 16:31
"""

handler = None
handler: ModbusSingleRequestHandler = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Handler can be all 3 handlers, but I am ok with only typing one of them.

Copy link
Collaborator

@janiversen janiversen left a comment

Choose a reason for hiding this comment

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

LGTM.

@janiversen janiversen merged commit e752b6d into dev Feb 27, 2023
@janiversen janiversen deleted the HA-mypy-config branch February 27, 2023 16:36
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants