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.

Sign up for GitHub

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

Eliminate more implicit optionals in transport #1873

Closed
wants to merge 6 commits into from
Closed

Eliminate more implicit optionals in transport #1873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ecb4438
Revert "More elegant noop. (#1859)"
alexrudd2 Oct 30, 2023
0cf8060
remove more implicit optional
alexrudd2 Oct 31, 2023
6504a85
Merge branch 'dev' into more-implicit-optional-transport
janiversen Oct 31, 2023
bdd1c53
Merge branch 'dev' into more-implicit-optional-transport
alexrudd2 Nov 8, 2023
4cd865e
Revert "Revert "More elegant noop. (#1859)""
alexrudd2 Nov 12, 2023
1456e85
mypy ignore
alexrudd2 Nov 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions pymodbus/transport/transport.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def __init__(
self.transport: asyncio.BaseTransport = None
self.loop: asyncio.AbstractEventLoop = None
self.recv_buffer: bytes = b""
self.call_create: Callable[[], Coroutine[Any, Any, Any]] = lambda: None
self.call_create: Callable[[], Coroutine[Any, Any, Any]] = lambda: None # type: ignore[assignment]
if self.is_server:
self.active_connections: dict[str, ModbusProtocol] = {}
else:
Expand Down Expand Up @@ -517,7 +517,7 @@ def __init__(self, protocol: ModbusProtocol, listen: int | None = None) -> None:
asyncio.DatagramTransport.__init__(self)
asyncio.Transport.__init__(self)
self.protocol: ModbusProtocol = protocol
self.other_modem: NullModem = None
self.other_modem: NullModem
self.listen = listen
self.manipulator: Callable[[bytes], list[bytes]] | None = None
self._is_closing = False
Expand Down Expand Up @@ -590,10 +590,10 @@ def close(self) -> None:
if self.connections:
with suppress(KeyError):
del self.connections[self]
if self.other_modem:
self.other_modem.other_modem = None
if hasattr(self, 'other_modem'):
del self.other_modem.other_modem
alexrudd2 marked this conversation as resolved.
Show resolved Hide resolved
self.other_modem.close()
self.other_modem = None
del self.other_modem
if self.protocol:
self.protocol.connection_lost(None)

Expand Down
4 changes: 2 additions & 2 deletions pymodbus/transport/transport_serial.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def setup(self):

def close(self, exc: Exception | None = None) -> None:
"""Close the transport gracefully."""
if not self.sync_serial:
if not hasattr(self, 'sync_serial'):
Copy link
Collaborator

Choose a reason for hiding this comment

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

see below please.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I understand your concern. Permitting sync_serial to be None means introducing more code to guard against calling it when None, so I found it cleanest to simply del the whole object.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or telling mypy to keep silent !!! we do not add code, just to please mypy.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My experience is actually different a lot of the juniors do not understand nor agree with type checking at all....

There are many python developers who see the lack of type checking as a big advantage...being a C/C++/C# I am very used to strict type checking, but have also seen how it can make code a lot more complicated.

Personally I think the bridge (free use / type checking) python tries to build might end being a split into 2 products.

I like the freedom python allows, but see that mypy catches many problems....so for me a balance is the correct choice, meaning think if what mypy suggest mamés the code better if not ask mypy to stop complaining.

return
with contextlib.suppress(Exception):
self.sync_serial.flush()
Expand All @@ -51,7 +51,7 @@ def close(self, exc: Exception | None = None) -> None:
else:
self.async_loop.remove_reader(self.sync_serial.fileno())
self.sync_serial.close()
self.sync_serial = None
del self.sync_serial
janiversen marked this conversation as resolved.
Show resolved Hide resolved
if exc:
with contextlib.suppress(Exception):
self._protocol.connection_lost(exc)
Expand Down