pylint error fixes, and temporary pylint suppressions to be removed a…

…fter merge

.github/workflows/run_tests.yml

@@ -132,8 +132,8 @@ jobs:
           make check-mypy
 
       # TODO: Re-enable static analysis once API is firmed up
-      # - name: Run Pylint
-      #   run: make check-lint
+       - name: Run Pylint
+        run: make check-lint
 
       # Run isort/black style check
       - name: Run isort

smartsim/_core/_cli/build.py

@@ -27,14 +27,12 @@
 import argparse
 import os
 import platform
-import sys
 import typing as t
-from pathlib import Path
 
 from tabulate import tabulate
 
 from smartsim._core._cli.scripts.dragon_install import install_dragon
-from smartsim._core._cli.utils import SMART_LOGGER_FORMAT, color_bool, pip
+from smartsim._core._cli.utils import SMART_LOGGER_FORMAT, pip
 from smartsim._core._install import builder
 from smartsim._core._install.buildenv import (
     BuildEnv,
@@ -45,11 +43,15 @@
 )
 from smartsim._core._install.builder import BuildError, Device
 from smartsim._core.config import CONFIG
-from smartsim.error import SSConfigError
 from smartsim.log import get_logger
 
 logger = get_logger("Smart", fmt=SMART_LOGGER_FORMAT)
 
+# ***************************************
+# TODO: Remove pylint disable after merge
+# ***************************************
+# pylint: disable=too-many-statements,unused-variable,no-value-for-parameter,no-member,invalid-name,condition-evals-to-constant,unused-variable
+
 # NOTE: all smartsim modules need full paths as the smart cli
 #       may be installed into a different directory.
 

smartsim/_core/_cli/cli.py

@@ -35,7 +35,6 @@
 from smartsim._core._cli.clean import configure_parser as clean_parser
 from smartsim._core._cli.clean import execute as clean_execute
 from smartsim._core._cli.clean import execute_all as clobber_execute
-from smartsim._core._cli.dbcli import execute as dbcli_execute
 from smartsim._core._cli.info import execute as info_execute
 from smartsim._core._cli.plugin import plugins
 from smartsim._core._cli.site import execute as site_execute

smartsim/_core/_install/builder.py

@@ -26,7 +26,6 @@
 
 # pylint: disable=too-many-lines
 
-import concurrent.futures
 import enum
 import fileinput
 import itertools
@@ -36,7 +35,6 @@
 import shutil
 import stat
 import subprocess
-import sys
 import tarfile
 import tempfile
 import typing as t
@@ -117,6 +115,7 @@ class Platform(t.NamedTuple):
     architecture: Architecture
 
 
+# TODO: Add FeatureStoreBuilder member
 class Builder:
     """Base class for building third-party libraries"""
 

smartsim/_core/commands/command.py

@@ -89,9 +89,9 @@ def __len__(self) -> int:
         """Get the length of the command list."""
         return len(self._command)
 
-    def insert(self, idx: int, value: str) -> None:
+    def insert(self, index: int, value: str) -> None:
         """Insert a command at the specified index."""
-        self._command.insert(idx, value)
+        self._command.insert(index, value)
 
     def __str__(self) -> str:  # pragma: no cover
         string = f"\nCommand: {' '.join(str(cmd) for cmd in self.command)}"

smartsim/_core/commands/command_list.py

@@ -83,7 +83,8 @@ def __setitem__(
                 isinstance(item, str) for item in sublist.command
             ):
                 raise TypeError(
-                    "Value sublists must be a list of Commands when assigning to a slice"
+                    "Value sublists must be a list of Commands when \
+assigning to a slice"
                 )
         self._commands[idx] = (deepcopy(val) for val in value)
 
@@ -95,9 +96,9 @@ def __len__(self) -> int:
         """Get the length of the Command list."""
         return len(self._commands)
 
-    def insert(self, idx: int, value: Command) -> None:
+    def insert(self, index: int, value: Command) -> None:
         """Insert a Command at the specified index."""
-        self._commands.insert(idx, value)
+        self._commands.insert(index, value)
 
     def __str__(self) -> str:  # pragma: no cover
         string = "\n\nCommand List:\n\n"

smartsim/_core/commands/launch_commands.py

@@ -40,11 +40,9 @@ def postlaunch_command(self) -> CommandList:
 
     def __str__(self) -> str:  # pragma: no cover
         string = "\n\nPrelaunch Command List:\n"
-        for pre_cmd in self.prelaunch_command:
-            string += f"{pre_cmd}\n"
+        string += f"\n{' '.join(str(pre_cmd) for pre_cmd in self.prelaunch_command)}"
         string += "\n\nLaunch Command List:\n"
-        for launch_cmd in self.launch_command:
-            string += f"{launch_cmd}\n"
+        string += f"\n{' '.join(str(launch_cmd) for launch_cmd in self.launch_command)}"
         string += "\n\nPostlaunch Command List:\n"
         for post_cmd in self.postlaunch_command:
             string += f"{post_cmd}\n"

smartsim/_core/config/config.py

@@ -32,9 +32,6 @@
 
 import psutil
 
-from ...error import SSConfigError
-from ..utils.helpers import expand_exe_path
-
 # Configuration Values
 #
 # These values can be set through environment variables to

smartsim/_core/control/job.py

@@ -29,8 +29,6 @@
 import typing as t
 from dataclasses import dataclass
 
-from smartsim.entity._mock import Mock
-
 from ...entity import SmartSimEntity
 from ...status import JobStatus
 
@@ -78,7 +76,8 @@ def __init__(self) -> None:
 
     @property
     def is_fs(self) -> bool:
-        """Returns `True` if the entity represents a feature store or feature store shard"""
+        """Returns `True` if the entity represents a feature store or
+        feature store shard"""
         return self.type in ["featurestore", "fsnode"]
 
     @property

smartsim/_core/control/manifest.py

@@ -29,8 +29,6 @@
 import typing as t
 from dataclasses import dataclass, field
 
-from smartsim.entity._mock import Mock
-
 from ...builders import Ensemble
 from ...database import FeatureStore
 from ...entity import Application, FSNode, SmartSimEntity

smartsim/_core/dispatch.py

@@ -27,7 +27,6 @@
 from __future__ import annotations
 
 import dataclasses
-import os
 import pathlib
 import typing as t
 

smartsim/_core/entrypoints/dragon_client.py

@@ -148,6 +148,7 @@ def execute_entrypoint(args: DragonClientEntrypointArgs) -> int:
 
     requests.append(DragonShutdownRequest(immediate=False, frontend_shutdown=True))
 
+    # TODO: DragonConnector constructor needs a path
     connector = DragonConnector()
 
     for request in requests:

smartsim/_core/generation/generator.py

@@ -52,18 +52,19 @@ class _GenerableProtocol(t.Protocol):
     and parameters."""
 
     files: FileSysOperationSet
-    # TODO change when file_parameters taken off Application during Ensemble refactor ticket
+    # TODO change when file_parameters taken off Application
+    # during Ensemble refactor ticket
     file_parameters: t.Mapping[str, str]
 
 
-Job_Path = namedtuple("Job_Path", ["run_path", "out_path", "err_path"])
+JobPath = namedtuple("JobPath", ["run_path", "out_path", "err_path"])
 """Namedtuple that stores a Job's run directory, output file path, and
 error file path."""
 
 
 class Generator:
-    """The Generator class creates the directory structure for a SmartSim Job by building
-    and executing file operation commands.
+    """The Generator class creates the directory structure for a
+    SmartSim Job by building and executing file operation commands.
     """
 
     run_directory = "run"
@@ -74,8 +75,8 @@ class Generator:
     def __init__(self, root: pathlib.Path) -> None:
         """Initialize a Generator object
 
-        The Generator class is responsible for constructing a Job's directory, performing
-        the following tasks:
+        The Generator class is responsible for constructing a
+        Job's directory, performing the following tasks:
 
         - Creating the run and log directories
         - Generating the output and error files
@@ -85,7 +86,8 @@ def __init__(self, root: pathlib.Path) -> None:
         :param root: The base path for job-related files and directories
         """
         self.root = root
-        """The root directory under which all generated files and directories will be placed."""
+        """The root directory under which all generated files and 
+        directories will be placed."""
 
     def _build_job_base_path(self, job: Job, job_index: int) -> pathlib.Path:
         """Build and return a Job's base directory. The path is created by combining the
@@ -102,9 +104,9 @@ def _build_job_base_path(self, job: Job, job_index: int) -> pathlib.Path:
         return pathlib.Path(job_path)
 
     def _build_job_run_path(self, job: Job, job_index: int) -> pathlib.Path:
-        """Build and return a Job's run directory. The path is formed by combining
-        the base directory with the `run_directory` class-level constant, which specifies
-        the name of the Job's run folder.
+        """Build and return a Job's run directory. The path is formed by
+        combining the base directory with the `run_directory` class-level
+        constant, which specifies the name of the Job's run folder.
 
         :param job: Job object
         :param job_index: Job index
@@ -115,8 +117,8 @@ def _build_job_run_path(self, job: Job, job_index: int) -> pathlib.Path:
 
     def _build_job_log_path(self, job: Job, job_index: int) -> pathlib.Path:
         """Build and return a Job's log directory. The path is formed by combining
-        the base directory with the `log_directory` class-level constant, which specifies
-        the name of the Job's log folder.
+        the base directory with the `log_directory` class-level constant,
+        which specifies the name of the Job's log folder.
 
         :param job: Job object
         :param job_index: Job index
@@ -137,8 +139,9 @@ def _build_log_file_path(log_path: pathlib.Path) -> pathlib.Path:
 
     @staticmethod
     def _build_out_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path:
-        """Build and return the path to the output file. The path is created by combining
-        the Job's log directory with the job name and appending the `.out` extension.
+        """Build and return the path to the output file.
+        The path is created by combining the Job's log directory with the
+        job name and appending the `.out` extension.
 
         :param log_path: Path to log directory
         :param job_name: Name of the Job
@@ -159,7 +162,7 @@ def _build_err_file_path(log_path: pathlib.Path, job_name: str) -> pathlib.Path:
         err_file_path = log_path / f"{job_name}.err"
         return err_file_path
 
-    def generate_job(self, job: Job, job_index: int) -> Job_Path:
+    def generate_job(self, job: Job, job_index: int) -> JobPath:
         """Build and return the Job's run directory, output file, and error file.
 
         This method creates the Job's run and log directories, generates the
@@ -189,21 +192,21 @@ def generate_job(self, job: Job, job_index: int) -> Job_Path:
             dt_string = datetime.now().strftime("%d/%m/%Y %H:%M:%S")
             log_file.write(f"Generation start date and time: {dt_string}\n")
 
-        return Job_Path(job_path, out_file, err_file)
+        return JobPath(job_path, out_file, err_file)
 
     @classmethod
     def _build_commands(
         cls,
-        entity: entity.SmartSimEntity,
+        smartsim_entity: entity.SmartSimEntity,
         job_path: pathlib.Path,
         log_path: pathlib.Path,
     ) -> CommandList:
         """Build file operation commands for a Job's entity.
 
-        This method constructs commands for copying, symlinking, and writing tagged files
-        associated with the Job's entity. This method builds the constructs the commands to
-        generate the Job's run and log directory. It aggregates these commands into a CommandList
-        to return.
+        This method constructs commands for copying, symlinking, and writing
+        tagged files associated with the Job's entity. This method builds
+        the constructs the commands to generate the Job's run and log directory.
+        It aggregates these commands into a CommandList to return.
 
         :param job: Job object
         :param job_path: The file path for the Job run folder
@@ -215,8 +218,8 @@ def _build_commands(
 
         cls._append_mkdir_commands(cmd_list, job_path, log_path)
 
-        if isinstance(entity, _GenerableProtocol):
-            cls._append_file_operations(cmd_list, entity, context)
+        if isinstance(smartsim_entity, _GenerableProtocol):
+            cls._append_file_operations(cmd_list, smartsim_entity, context)
 
         return cmd_list
 
@@ -237,23 +240,27 @@ def _append_mkdir_commands(
     def _append_file_operations(
         cls,
         cmd_list: CommandList,
-        entity: _GenerableProtocol,
+        smartsim_entity: _GenerableProtocol,
         context: GenerationContext,
     ) -> None:
         """Append file operation Commands (copy, symlink, configure) for all
         files attached to the entity.
 
         :param cmd_list: A CommandList object containing the commands to be executed
-        :param entity: The Job's attached entity
+        :param smartsim_entity: The Job's attached entity
         :param context: A GenerationContext object that holds the Job's run directory
         """
-        copy_ret = cls._copy_files(entity.files.copy_operations, context)
+        copy_ret = cls._copy_files(smartsim_entity.files.copy_operations, context)
         cmd_list.extend(copy_ret)
 
-        symlink_ret = cls._symlink_files(entity.files.symlink_operations, context)
+        symlink_ret = cls._symlink_files(
+            smartsim_entity.files.symlink_operations, context
+        )
         cmd_list.extend(symlink_ret)
 
-        configure_ret = cls._configure_files(entity.files.configure_operations, context)
+        configure_ret = cls._configure_files(
+            smartsim_entity.files.configure_operations, context
+        )
         cmd_list.extend(configure_ret)
 
     @classmethod
@@ -266,7 +273,7 @@ def _execute_commands(cls, cmd_list: CommandList) -> None:
         :param cmd_list: A CommandList object containing the commands to be executed
         """
         for cmd in cmd_list:
-            subprocess.run(cmd.command)
+            subprocess.run(cmd.command, check=False)
 
     @staticmethod
     def _mkdir_file(file_path: pathlib.Path) -> Command:

smartsim/_core/launcher/dragon/dragon_launcher.py

@@ -69,6 +69,11 @@
 
 logger = get_logger(__name__)
 
+# ***************************************
+# TODO: Remove pylint disable after merge
+# ***************************************
+# pylint: disable=protected-access
+
 
 class DragonLauncher(WLMLauncher):
     """This class encapsulates the functionality needed
@@ -369,13 +374,15 @@ def _assert_schema_type(obj: object, typ: t.Type[_SchemaT], /) -> _SchemaT:
     return obj
 
 
-from smartsim._core.dispatch import dispatch
+from smartsim._core.dispatch import dispatch  # pylint: disable=wrong-import-position
 
 # >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
 # TODO: Remove this registry and move back to builder file after fixing
 #       circular import caused by `DragonLauncher.supported_rs`
 # -----------------------------------------------------------------------------
-from smartsim.settings.arguments.launch.dragon import DragonLaunchArguments
+from smartsim.settings.arguments.launch.dragon import (  # pylint: disable=wrong-import-position
+    DragonLaunchArguments,
+)
 
 
 def _as_run_request_args_and_policy(

smartsim/_core/launcher/step/dragon_step.py

@@ -48,6 +48,7 @@
 logger = get_logger(__name__)
 
 
+# pylint: disable=too-many-function-args
 class DragonStep(Step):
     def __init__(self, name: str, cwd: str, run_settings: DragonRunSettings) -> None:
         """Initialize a srun job step

smartsim/_core/launcher/step/sge_step.py

@@ -33,6 +33,9 @@
 logger = get_logger(__name__)
 
 
+# pylint: disable=too-many-function-args
+
+
 class SgeQsubBatchStep(Step):
     def __init__(
         self, name: str, cwd: str, batch_settings: SgeQsubBatchSettings