Repository: jupyter/jupyter_client
Pull request #986 (2023-10-23 12:25:07)

Unused `*args` in `KernelManager`'s `__init__`
I think that if it is unused, then it is likely a mistake from the caller that meant to pass a kwarg.

Should it maybe get a deprectation period ?

blink1073: Yeah I think it should be deprecated and removed in 9.0.  
Carreau: Rebased on main, should fix linting.

Patch:
diff --git a/jupyter_client/manager.py b/jupyter_client/manager.py
index 6131838..088acd6 100644
--- a/jupyter_client/manager.py
+++ b/jupyter_client/manager.py
@@ -9,6 +9,7 @@ import signal
 import sys
 import typing as t
 import uuid
+import warnings
 from asyncio.futures import Future
 from concurrent.futures import Future as CFuture
 from contextlib import contextmanager
@@ -19,6 +20,7 @@ from jupyter_core.utils import run_sync
 from traitlets import (
     Any,
     Bool,
+    Dict,
     DottedObjectName,
     Float,
     Instance,
@@ -106,6 +108,15 @@ class KernelManager(ConnectionFileMixin):
 
     def __init__(self, *args: t.Any, **kwargs: t.Any) -> None:
         """Initialize a kernel manager."""
+        if args:
+            warnings.warn(
+                "Passing positional only arguments to "
+                "`KernelManager.__init__` is deprecated since jupyter_client"
+                " 8.6, and will become an error on future versions. Positional "
+                " arguments have been ignored since jupyter_client 7.0",
+                DeprecationWarning,
+                stacklevel=2,
+            )
         self._owns_kernel = kwargs.pop("owns_kernel", True)
         super().__init__(**kwargs)
         self._shutdown_status = _ShutdownStatus.Unset
@@ -206,7 +217,7 @@ class KernelManager(ConnectionFileMixin):
         return self.kernel_name in {"python", "python2", "python3"}
 
     # Protected traits
-    _launch_args: Any = Any()
+    _launch_args: t.Optional["Dict[str, Any]"] = Dict(allow_none=True)
     _control_socket: Any = Any()
 
     _restarter: Any = Any()
@@ -281,7 +292,13 @@ class KernelManager(ConnectionFileMixin):
 
         .. version-added: 8.5
         """
-        self._launch_args["env"].update(env)
+        # Mypy think this is unreachable as it see _launch_args as Dict, not t.Dict
+        if (
+            isinstance(self._launch_args, dict)
+            and "env" in self._launch_args
+            and isinstance(self._launch_args["env"], dict)  # type: ignore [unreachable]
+        ):
+            self._launch_args["env"].update(env)  # type: ignore [unreachable]
 
     def format_kernel_cmd(self, extra_arguments: t.Optional[t.List[str]] = None) -> t.List[str]:
         """Replace templated args (e.g. {connection_file})"""
@@ -307,13 +324,14 @@ class KernelManager(ConnectionFileMixin):
         # is not usable by non python kernels because the path is being rerouted when
         # inside of a store app.
         # See this bug here: https://bugs.python.org/issue41196
-        ns = {
+        ns: t.Dict[str, t.Any] = {
             "connection_file": os.path.realpath(self.connection_file),
             "prefix": sys.prefix,
         }
 
         if self.kernel_spec:  # type:ignore[truthy-bool]
             ns["resource_dir"] = self.kernel_spec.resource_dir
+        assert isinstance(self._launch_args, dict)
 
         ns.update(self._launch_args)
 
@@ -371,7 +389,8 @@ class KernelManager(ConnectionFileMixin):
         self.shutting_down = False
         self.kernel_id = self.kernel_id or kw.pop("kernel_id", str(uuid.uuid4()))
         # save kwargs for use in restart
-        self._launch_args = kw.copy()
+        # assigning Traitlets Dicts to Dict make mypy unhappy but is ok
+        self._launch_args = kw.copy()  # type:ignore [assignment]
         if self.provisioner is None:  # will not be None on restarts
             self.provisioner = KPF.instance(parent=self.parent).create_provisioner_instance(
                 self.kernel_id,
diff --git a/jupyter_client/multikernelmanager.py b/jupyter_client/multikernelmanager.py
index 122db74..d14a3f8 100644
--- a/jupyter_client/multikernelmanager.py
+++ b/jupyter_client/multikernelmanager.py
@@ -223,7 +223,7 @@ class MultiKernelManager(LoggingConfigurable):
         .. version-added: 8.5
         """
         if kernel_id in self:
-            self._kernels[kernel_id].update_env(env)
+            self._kernels[kernel_id].update_env(env=env)
 
     async def _add_kernel_when_ready(
         self, kernel_id: str, km: KernelManager, kernel_awaitable: t.Awaitable


Test patch:
diff --git a/tests/test_manager.py b/tests/test_manager.py
index e3d6ea2..23c246a 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -32,3 +32,33 @@ def test_connection_file_real_path():
         km._launch_args = {}
         cmds = km.format_kernel_cmd()
         assert cmds[4] == "foobar"
+
+
+def test_env_update_launch_args_not_set():
+    km = KernelManager()
+    km.update_env(env={"A": "A"})
+
+
+def test_env_update_launch_args_not_dict():
+    km = KernelManager()
+    km._launch_args = None
+    km.update_env(env={"B": "B"})
+
+
+def test_env_update_launch_args_no_env():
+    km = KernelManager()
+    km._launch_args = {}
+    km.update_env(env={"C": "C"})
+
+
+def test_env_update_launch_args_env_not_dict():
+    km = KernelManager()
+    km._launch_args = {"env": None}
+    km.update_env(env={"D": "D"})
+
+
+def test_env_update_launch_args_env_dic():
+    km = KernelManager()
+    km._launch_args = {"env": {}}
+    km.update_env(env={"E": "E"})
+    assert km._launch_args["env"]["E"] == "E"