Merge pull request #261 from SpiNNakerManchester/pylint_strict

make Pylint checks much stricter

.github/workflows/python_actions.yml

@@ -64,6 +64,7 @@ jobs:
       uses: ./support/actions/pylint
       with: 
         package: ${{ env.ROOT_PKG }}
+        exitcheck: 31 # Action fails on any message
         language: en_GB
     - name: Lint with mypy
       run: mypy $ROOT_PKG

.pylint_dict.txt

@@ -18,14 +18,24 @@ Zenodo
 LCM
 
 # Python types
-RawConfigParser
 AbstractBase
 LogCapture
+RawConfigParser
 ZipFile
 
 # Python packages
-# numpy
-# mypy
+numpy
+mypy
+
+# Our "special" words
+aplx
+config
+configs
+metaclasses
+
+# Python bits
+iter
+UnusedVariable
 
 # Misc
 haha

.pylintrc

@@ -68,15 +68,20 @@ confidence=
 # --enable=similarities". If you want to run only the classes checker, but have
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
-disable=R,C,unsubscriptable-object
+disable=consider-using-dict-items, missing-module-docstring, unsubscriptable-object
+
+# consider-using-dict-items "for x in foo:"  is fine!
+
+# missing-module-docstring expects a comment at import level which we don't do
+
 # False positives for unsubscriptable-object. Mypy better at this class of issue
 # See https://github.com/pylint-dev/pylint/issues/1498
 
 # Enable the message, report, category or checker with the given id(s). You can
 # either give multiple identifier separated by comma (,) or put this option
 # multiple time (only on the command line, not in the configuration file where
 # it should appear only once). See also the "--disable" option for examples.
-enable=c-extension-no-member,C0402,C0403
+enable=
 
 
 [REPORTS]

spinn_utilities/abstract_base.py

@@ -40,6 +40,7 @@ def my_abstract_method(self, ...):
     return funcobj
 
 
+#  pylint: disable=invalid-name
 class abstractproperty(property):
     """
     A decorator indicating abstract properties.
@@ -110,9 +111,9 @@ def my_abstract_method(self, ...):
                 ...
     """
 
-    def __new__(cls, name, bases, namespace, **kwargs):
+    def __new__(mcs, name, bases, namespace, **kwargs):
         # Actually make the class
-        abs_cls = super().__new__(cls, name, bases, namespace, **kwargs)
+        abs_cls = super().__new__(mcs, name, bases, namespace, **kwargs)
 
         # Get set of abstract methods from namespace
         abstracts = set(nm for nm, val in namespace.items()

spinn_utilities/abstract_context_manager.py

@@ -12,10 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from .abstract_base import AbstractBase, abstractmethod
 from types import TracebackType
 from typing import Optional, Type
 from typing_extensions import Literal, Self
+from .abstract_base import AbstractBase, abstractmethod
 
 
 class AbstractContextManager(object, metaclass=AbstractBase):

spinn_utilities/bytestring_utils.py

@@ -46,4 +46,5 @@ def as_hex(byte_string: bytes, start: Optional[int] = None,
     :return: Comma-separated hex values
     :rtype: str
     """
+    # pylint: disable=consider-using-f-string
     return ','.join('%02x' % i for i in iter(byte_string[start:end]))

spinn_utilities/citation/citation_aggregator.py

@@ -41,6 +41,8 @@
 
 CITATION_DOI_TYPE = 'identifier'
 
+# pylint: skip-file
+
 
 class CitationAggregator(object):
     """

spinn_utilities/citation/citation_updater_and_doi_generator.py

@@ -44,6 +44,8 @@
 AUTHOR_ORCID = "orcid"
 IDENTIFIER = 'identifier'
 
+# pylint: skip-file
+
 
 class _ZenodoException(Exception):
     """

spinn_utilities/conf_loader.py

@@ -12,12 +12,15 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import appdirs
+
 from configparser import NoOptionError
 import logging
 import os
 from typing import Callable, Dict, List, Sequence, Tuple, Union
+
+import appdirs
 from typing_extensions import TypeAlias
+
 from spinn_utilities import log
 from spinn_utilities.configs import (
     CamelCaseConfigParser, ConfigTemplateException,
@@ -26,7 +29,7 @@
 _SectionParser: TypeAlias = Callable[[CamelCaseConfigParser], None]
 
 
-def install_cfg_and_IOError(
+def install_cfg_and_error(
         filename: str, defaults: List[str],
         config_locations: List[str]) -> NoConfigFoundException:
     """
@@ -232,7 +235,7 @@ def load_config(
     config_locations = _config_locations(filename)
     if not any(os.path.isfile(f) for f in config_locations):
         if defaults:
-            raise install_cfg_and_IOError(
+            raise install_cfg_and_error(
                 filename, defaults, config_locations)
         else:
             logger.error("No default cfg files provided")

spinn_utilities/config_holder.py

@@ -76,7 +76,7 @@ def _pre_load_config() -> CamelCaseConfigParser:
 
     :raises ConfigException: Raise if called before setup
     """
-    # If you getthis error during a unittest then unittest_step was not called
+    # If you get this error during a unit test, unittest_step was not called
     if not __unittest_mode:
         raise ConfigException(
             "Accessing config values before setup is not supported")

spinn_utilities/config_setup.py

@@ -36,4 +36,7 @@ def unittest_setup() -> None:
 
 
 def add_spinn_utilities_cfg() -> None:
+    """
+    Loads the default config values for spinn_utilities
+    """
     add_default_cfg(os.path.join(os.path.dirname(__file__), BASE_CONFIG_FILE))

spinn_utilities/configs/camel_case_config_parser.py

@@ -22,6 +22,9 @@
 
 
 class CamelCaseConfigParser(configparser.RawConfigParser):
+    """
+    Extends the Parser to allow for differences in case and underscores
+    """
     __slots__ = ["_read_files"]
 
     def optionxform(self, optionstr: str) -> str:

spinn_utilities/data/reset_status.py

@@ -21,8 +21,8 @@ class ResetStatus(Enum):
 
     This class is design to used internally by UtilsDataView.
     """
-    NOT_SETUP = (0)
-    SETUP = (1)
-    HAS_RUN = (2)
-    SOFT_RESET = (3)
-    HARD_RESET = (4)
+    NOT_SETUP = 0
+    SETUP = 1
+    HAS_RUN = 2
+    SOFT_RESET = 3
+    HARD_RESET = 4

spinn_utilities/data/run_status.py

@@ -27,9 +27,9 @@ class RunStatus(Enum):
 
     This class is design to used internally by :py:class:`UtilsDataView`.
     """
-    NOT_SETUP = (0)
-    NOT_RUNNING = (12)
-    IN_RUN = (2)
-    STOP_REQUESTED = (3)
-    STOPPING = (4)
-    SHUTDOWN = (5)
+    NOT_SETUP = 0
+    NOT_RUNNING = 1
+    IN_RUN = 2
+    STOP_REQUESTED = 3
+    STOPPING = 4
+    SHUTDOWN = 5

spinn_utilities/data/utils_data_view.py

@@ -16,14 +16,14 @@
 from typing import List, Optional
 
 from unittest import SkipTest
-from .data_status import DataStatus
-from .reset_status import ResetStatus
-from .run_status import RunStatus
 from spinn_utilities.exceptions import (
     SpiNNUtilsException,
     SimulatorNotSetupException, SimulatorRunningException,
     SimulatorShutdownException, UnexpectedStateChange)
 from spinn_utilities.executable_finder import ExecutableFinder
+from .data_status import DataStatus
+from .reset_status import ResetStatus
+from .run_status import RunStatus
 # pylint: disable=protected-access
 
 

spinn_utilities/data/utils_data_writer.py

@@ -18,12 +18,12 @@
 from spinn_utilities.exceptions import (
     IllegalWriterException, InvalidDirectory, SimulatorNotRunException,
     UnexpectedStateChange)
+from spinn_utilities.executable_finder import ExecutableFinder
 from spinn_utilities.log import FormatAdapter
 from .data_status import DataStatus
 from .reset_status import ResetStatus
 from .run_status import RunStatus
 from .utils_data_view import UtilsDataView, _UtilsDataModel
-from spinn_utilities.executable_finder import ExecutableFinder
 
 logger = FormatAdapter(logging.getLogger(__file__))
 # pylint: disable=protected-access

spinn_utilities/executable_finder.py

@@ -130,6 +130,11 @@ def get_executable_paths(self, executable_names: str) -> List[str]:
         return results
 
     def check_logs(self) -> None:
+        """
+        Compares the aplx files used against the ones available for use
+
+        Reports the aplx files never used.
+        """
         if not self._paths_log:
             print("environ GLOBAL_REPORTS not set!")
             return
@@ -167,6 +172,9 @@ def check_logs(self) -> None:
                 print(binary)
 
     def clear_logs(self) -> None:
+        """
+        Deletes log files from previous runs
+        """
         if not self._paths_log:
             print("environ GLOBAL_REPORTS not set!")
             return

spinn_utilities/log.py

@@ -59,7 +59,7 @@ class ConfiguredFormatter(logging.Formatter):
     """
     Defines the logging format for the SpiNNaker host software.
     """
-    # Precompile this RE; it gets used quite a few times
+    # Pre-compile this RE; it gets used quite a few times
     __last_component = re.compile(r'\.[^.]+$')
 
     def __init__(self, conf):
@@ -197,6 +197,10 @@ def set_kill_level(cls, level: Optional[int] = None):
 
     @classmethod
     def set_log_store(cls, log_store: Optional[LogStore]):
+        """
+        Sets a Object to write the log messages to
+        :param LogStore log_store:
+        """
         if log_store is not None and not isinstance(log_store, LogStore):
             raise TypeError("log_store must be a LogStore")
         cls.__log_store = log_store
@@ -258,6 +262,9 @@ def process(self, msg: object, kwargs) -> Tuple[object, dict]:
 
     @classmethod
     def atexit_handler(cls) -> None:
+        """
+        Adds code to print out high level log messages python run ends
+        """
         messages = []
         if cls.__log_store:
             try:

spinn_utilities/make_tools/converter.py

@@ -80,6 +80,7 @@ def _mkdir(destination):
 if __name__ == '__main__':
     _src = sys.argv[1]
     _dest = sys.argv[2]
+    #  pylint: disable=invalid-name
     if len(sys.argv) > 3:
         _new_dict = bool(sys.argv[3])
     else: