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

enable mypy --check-untyped-defs #2096

Merged
merged 1 commit into from
Mar 7, 2024
Merged

enable mypy --check-untyped-defs #2096

merged 1 commit into from
Mar 7, 2024

Conversation

alexrudd2
Copy link
Collaborator

image

When I started, there were over >150 mypy errors in untyped definitions. They have all been slowly solved.

Remaining ignores:

  • for diag_message, pyright is smart enough to infer int, but mypy is not. Actually, this code has a lot of type problems and needs a closer look
  • ascii_framer is going to be deleted soon. (See Simplify packet construction #2086)
  • the transaction code also has messy types but is being reworked

@janiversen
Copy link
Collaborator

The ascii_framer is not being removed.

Be very careful when you rework transaction, it has a lot of strange (but needed) code....and it seems I cut a little bit to deep, when removing a function in rtu_framer.

The request/response classes surely needs a refactoring, but that is not on my short list.

@alexrudd2
Copy link
Collaborator Author

The ascii_framer is not being removed.

I meant the type error (which is now already gone).

Be very careful when you rework transaction, it has a lot of strange (but needed) code....and it seems I cut a little bit to deep, when removing a function in rtu_framer.

Yeah, your fixing the code satisfied mypy. I do not intend to do further changes right now. My goal of --no-untyped-defs is achieved with this PR.

@alexrudd2
Copy link
Collaborator Author

The request/response classes surely needs a refactoring, but that is not on my short list.

I believe this is already mentioned in #1034

@janiversen
Copy link
Collaborator

No #1034 is specifically about DIAG, my comment was about all the request/response classes.

I am f.x. convinced it would make the code simpler not to have separate a request and response class for each function code, but as wrote that is for later...right now my focus is the message layer, which is to followed by a new transaction layer with proper locking.

@alexrudd2
Copy link
Collaborator Author

@laundmo has the typing situation improved for you with these recent changes?

For me all the mypy errors are cleared, and about 50% of the pyright ones.

@janiversen janiversen merged commit 27b2915 into dev Mar 7, 2024
1 check passed
@janiversen janiversen deleted the check-untyped-defs branch March 7, 2024 20:06
janiversen pushed a commit to dnssoftware/pymodbus that referenced this pull request Mar 17, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
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.

2 participants