Skip to content

Commit

Permalink
Remove Problematic Type Union (#694)
Browse files Browse the repository at this point in the history
In the shell launcher, an unintended and unused type union sunk into the
``ShellLauncherCommand`` type for the ``command_tuple`` type, allowing
the attr to be of type ``Sequence[str]`` or ``tuple[str, tuple[str,
...]]``. The former works as expected, but the latter would error at
runtime when sent to the ``ShellLauncher`` when opening a subprocess.

```py
class ShellLauncher:
    ...
    def start(self, shell_command: ShellLauncherCommand) -> LaunchedJobID:
        ...
        exe, *rest = shell_command.command_tuple
        #     ^^^^ Mypy thinks this is `list[str] | list[tuple[str]]`
        expanded_exe = helpers.expand_exe_path(exe)
        # pylint: disable-next=consider-using-with
        self._launched[id_] = sp.Popen(
            (expanded_exe, *rest),
       #    ^^^^^^^^^^^^^^^^^^^^^
       #    And inadvertently casts this to `tuple[Any]` which errors
       #    at runtime
            cwd=shell_command.path,
            env={k: v for k, v in shell_command.env.items() if v is not None},
            stdout=shell_command.stdout,
            stderr=shell_command.stderr,
        )
        ...
```

Because this type was not being used, it can simply be removed from the
union.
[ committed by @MattToast ]
[ reviewed by @amandarichardsonn ]
  • Loading branch information
MattToast authored Sep 5, 2024
1 parent 0175b6b commit c169878
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion smartsim/_core/shell/shellLauncher.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class ShellLauncherCommand(t.NamedTuple):
path: pathlib.Path
stdout: io.TextIOWrapper | int
stderr: io.TextIOWrapper | int
command_tuple: tuple[str, tuple[str, ...]] | t.Sequence[str]
command_tuple: t.Sequence[str]


def make_shell_format_fn(
Expand Down

0 comments on commit c169878

Please sign in to comment.