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.

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

make Pylint checks much stricter #261

Merged
merged 18 commits into from
Jan 8, 2024
Merged
1 change: 1 addition & 0 deletions .github/workflows/python_actions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 13 additions & 3 deletions .pylint_dict.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 7 additions & 2 deletions .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
5 changes: 3 additions & 2 deletions spinn_utilities/abstract_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ def my_abstract_method(self, ...):
return funcobj


# pylint: disable=invalid-name
class abstractproperty(property):
"""
A decorator indicating abstract properties.
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion spinn_utilities/abstract_context_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions spinn_utilities/bytestring_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]))
2 changes: 2 additions & 0 deletions spinn_utilities/citation/citation_aggregator.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

CITATION_DOI_TYPE = 'identifier'

# pylint: skip-file


class CitationAggregator(object):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
AUTHOR_ORCID = "orcid"
IDENTIFIER = 'identifier'

# pylint: skip-file


class _ZenodoException(Exception):
"""
Expand Down
9 changes: 6 additions & 3 deletions spinn_utilities/conf_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
"""
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion spinn_utilities/config_holder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions spinn_utilities/config_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
3 changes: 3 additions & 0 deletions spinn_utilities/configs/camel_case_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions spinn_utilities/data/reset_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 6 additions & 6 deletions spinn_utilities/data/run_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 3 additions & 3 deletions spinn_utilities/data/utils_data_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion spinn_utilities/data/utils_data_writer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions spinn_utilities/executable_finder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion spinn_utilities/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions spinn_utilities/make_tools/converter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading