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

Fix type hints for http_server #1369

Merged
merged 3 commits into from
Feb 21, 2023
Merged

Fix type hints for http_server #1369

merged 3 commits into from
Feb 21, 2023

Conversation

alexrudd2
Copy link
Collaborator

Eliminates 3 more errors.

pymodbus/server/simulator/http_server.py:141: error: Argument 2 to "ModbusSimulatorContext" has incompatible type "str"; expected "Dict[str, Callable[..., Any]]"  [arg-type]
pymodbus/server/simulator/http_server.py:178: error: No overload variant of "__setitem__" of "list" matches argument types "int", "str"  [call-overload]
pymodbus/server/simulator/http_server.py:178: note: Possible overload variants:
pymodbus/server/simulator/http_server.py:178: note:     def __setitem__(self, SupportsIndex, Callable[[Any, Any], Any], /) -> None
pymodbus/server/simulator/http_server.py:178: note:     def __setitem__(self, slice, Iterable[Callable[[Any, Any], Any]], /) -> None
pymodbus/server/simulator/http_server.py:181: error: Need type annotation for "call_list" (hint: "call_list: List[<type>] = ...")  [var-annotated]

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.

Your changes to get mypy working are VERY useful, but please try not to remove/change code without understanding it.

Your work is really appreciated and I try my best to help you understand the code, but it seems this batch of review involved a lot more than just satisfying mypy (I think about the comments I left in the datastore as well).

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, thanks.

@janiversen janiversen merged commit 4d70ba5 into dev Feb 21, 2023
@janiversen janiversen deleted the http-mypy branch February 21, 2023 19:45
@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.

None yet

2 participants