Skip to content

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.

3 participants