From 481f1d2638c731a8692edc1ca1788c4cfd95e07f Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 15:59:26 +0100 Subject: [PATCH 01/10] Remove uses of pkg_resources This has finally disappeared in Python 3.11. These have been replaced by rather pedestrian path mangling from the 'rig' module filename. --- rig/machine_control/boot.py | 19 +++++++--- rig/machine_control/machine_controller.py | 13 +++++-- rig/place_and_route/place/sa/c_kernel.py | 6 +-- .../test_machine_controller.py | 38 ++++++++++++------- 4 files changed, 50 insertions(+), 26 deletions(-) diff --git a/rig/machine_control/boot.py b/rig/machine_control/boot.py index ad691f5..584d906 100644 --- a/rig/machine_control/boot.py +++ b/rig/machine_control/boot.py @@ -6,11 +6,12 @@ """ from . import struct_file, consts import enum -import pkg_resources import socket import struct import time +import os +import rig from rig.utils.docstrings import add_int_enums_to_docstring # Specifies the size of packets that should be sent to SpiNNaker to boot the @@ -102,10 +103,18 @@ def boot(hostname, boot_port=consts.BOOT_PORT, Layout of structs in memory. """ # Get the boot data if not specified. - scamp_binary = (scamp_binary if scamp_binary is not None else - pkg_resources.resource_filename("rig", "boot/scamp.boot")) - sark_struct = (sark_struct if sark_struct is not None else - pkg_resources.resource_filename("rig", "boot/sark.struct")) + if scamp_binary is None: # pragma: no branch + scamp_binary = os.path.join( + os.path.dirname(rig.__file__), + "boot", + "scamp.boot", + ) + if sark_struct is None: # pragma: no branch + sark_struct = os.path.join( + os.path.dirname(rig.__file__), + "boot", + "sark.struct", + ) with open(scamp_binary, "rb") as f: boot_data = f.read() diff --git a/rig/machine_control/machine_controller.py b/rig/machine_control/machine_controller.py index 1dfe7ca..5218357 100644 --- a/rig/machine_control/machine_controller.py +++ b/rig/machine_control/machine_controller.py @@ -8,9 +8,10 @@ import socket import struct import time -import pkg_resources import warnings +import rig + from rig.machine_control.consts import \ SCPCommands, NNCommands, NNConstants, AppFlags, LEDAction from rig.machine_control import boot, consts, regions, struct_file @@ -118,8 +119,14 @@ def __init__(self, initial_host, scp_port=consts.SCP_PORT, # Load default structs if none provided self.structs = structs if self.structs is None: - struct_data = pkg_resources.resource_string("rig", - "boot/sark.struct") + struct_data = open( + os.path.join( + os.path.dirname(rig.__file__), + "boot", + "sark.struct", + ), + "rb", + ).read() self.structs = struct_file.read_struct_file(struct_data) # This dictionary contains a lookup from chip (x, y) to the diff --git a/rig/place_and_route/place/sa/c_kernel.py b/rig/place_and_route/place/sa/c_kernel.py index 593bd84..02daef7 100644 --- a/rig/place_and_route/place/sa/c_kernel.py +++ b/rig/place_and_route/place/sa/c_kernel.py @@ -4,15 +4,13 @@ from six import iteritems -import pkg_resources - # An optional Rig dependency. from rig_c_sa import ffi import rig_c_sa - # Make sure installed rig_c_sa version is compatible. -pkg_resources.require("rig_c_sa>=0.3.1,<1.0.0") +rig_c_sa_version = tuple(map(int, rig_c_sa.__version__.split("."))) +assert (0, 3, 1) <= rig_c_sa_version < (1, 0, 0) class CKernel(object): diff --git a/tests/machine_control/test_machine_controller.py b/tests/machine_control/test_machine_controller.py index 5ed985b..6bbabf4 100644 --- a/tests/machine_control/test_machine_controller.py +++ b/tests/machine_control/test_machine_controller.py @@ -1,5 +1,4 @@ import mock -import pkg_resources import pytest import six from six import iteritems, itervalues @@ -10,6 +9,7 @@ import itertools import warnings +import rig from rig.machine_control.consts import ( SCPCommands, LEDAction, NNCommands, NNConstants) from rig.machine_control.machine_controller import ( @@ -27,6 +27,22 @@ from rig.routing_table import RoutingTableEntry, Routes +sark_struct_data = open( + os.path.join( + os.path.dirname(rig.__file__), + "boot", + "sark.struct", + ), + "rb", +).read() + +test_aplx_file = os.path.join( + os.path.dirname(rig.__file__), + "binaries", + "test.aplx", +) + + @pytest.fixture(scope="module") def controller(spinnaker_ip): return MachineController(spinnaker_ip) @@ -258,7 +274,7 @@ def test_load_application(self, controller, targets): assert len(controller.structs) > 0, \ "Controller has no structs, check test fixture." controller.load_application( - pkg_resources.resource_filename("rig", "binaries/test.aplx"), + test_aplx_file, targets, use_count=False ) @@ -314,8 +330,7 @@ def test_stop_signal(self, controller): controller.get_chip_info(0, 1).core_states[10] == consts.AppState.idle) controller.load_application( - pkg_resources.resource_filename("rig", - "binaries/test.aplx"), + test_aplx_file, {(0, 1): set([10])}, wait=True ) @@ -1389,8 +1404,7 @@ def test_read_struct_field(self, x, y, p, which_struct_ascii, field_ascii, field = six.b(field_ascii) # Open the struct file - struct_data = pkg_resources.resource_string("rig", "boot/sark.struct") - structs = struct_file.read_struct_file(struct_data) + structs = struct_file.read_struct_file(sark_struct_data) assert (which_struct in structs and field in structs[which_struct]), "Test is broken" @@ -1423,8 +1437,7 @@ def test_read_struct_field(self, x, y, p, which_struct_ascii, field_ascii, ]) def test_write_struct_field(self, x, y, p, which_struct, field, value): # Open the struct file - struct_data = pkg_resources.resource_string("rig", "boot/sark.struct") - structs = struct_file.read_struct_file(struct_data) + structs = struct_file.read_struct_file(sark_struct_data) assert (six.b(which_struct) in structs and six.b(field) in structs[six.b(which_struct)]), "Test is broken" @@ -1466,8 +1479,7 @@ def test_write_struct_field(self, x, y, p, which_struct, field, value): ) def test_read_vcpu_struct(self, x, y, p, vcpu_base, field, data, converted): - struct_data = pkg_resources.resource_string("rig", "boot/sark.struct") - structs = struct_file.read_struct_file(struct_data) + structs = struct_file.read_struct_file(sark_struct_data) vcpu_struct = structs[b"vcpu"] assert six.b(field) in vcpu_struct, "Test is broken" field_ = vcpu_struct[six.b(field)] @@ -1503,8 +1515,7 @@ def mock_read_struct_field(struct_name, field, x, y, p=0): ("rt_code", 8, b"\x08")] ) def test_write_vcpu_struct(self, x, y, p, vcpu_base, field, value, data): - struct_data = pkg_resources.resource_string("rig", "boot/sark.struct") - structs = struct_file.read_struct_file(struct_data) + structs = struct_file.read_struct_file(sark_struct_data) vcpu_struct = structs[b"vcpu"] assert six.b(field) in vcpu_struct, "Test is broken" field_ = vcpu_struct[six.b(field)] @@ -1532,8 +1543,7 @@ def mock_read_struct_field(struct_name, field, x, y, p=0): @pytest.mark.parametrize("x, y, p, vcpu_base", [(0, 1, 11, 0x67801234), (1, 4, 17, 0x33331110)]) def test_get_processor_status(self, x, y, p, vcpu_base): - struct_data = pkg_resources.resource_string("rig", "boot/sark.struct") - structs = struct_file.read_struct_file(struct_data) + structs = struct_file.read_struct_file(sark_struct_data) vcpu_struct = structs[b"vcpu"] # Create a mock SV struct reader From 0444bdd3f6b1a567e5251e038b40eb66fbd29e75 Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:03:10 +0100 Subject: [PATCH 02/10] Handle move of Iterable/Callable to collections.abc --- rig/machine_control/machine_controller.py | 3 ++- tests/machine_control/test_scp_connection.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/rig/machine_control/machine_controller.py b/rig/machine_control/machine_controller.py index 5218357..12f89cc 100644 --- a/rig/machine_control/machine_controller.py +++ b/rig/machine_control/machine_controller.py @@ -9,6 +9,7 @@ import struct import time import warnings +from six.moves.collections_abc import Iterable import rig @@ -1557,7 +1558,7 @@ def count_cores_in_state(self, state, app_id): an iterable of these, in which case the total count will be returned. """ - if (isinstance(state, collections.Iterable) and + if (isinstance(state, Iterable) and not isinstance(state, str)): # If the state is iterable then call for each state and return the # sum. diff --git a/tests/machine_control/test_scp_connection.py b/tests/machine_control/test_scp_connection.py index 9b0ea79..9a229e5 100644 --- a/tests/machine_control/test_scp_connection.py +++ b/tests/machine_control/test_scp_connection.py @@ -3,6 +3,7 @@ import pytest import struct import time +from six.moves.collections_abc import Callable from rig.machine_control.consts import \ SCPCommands, DataType, SDP_HEADER_LENGTH, RETRYABLE_SCP_RETURN_CODES, \ @@ -58,7 +59,7 @@ def test_scpcall(): assert call.arg1 == call.arg2 == call.arg3 == 0 assert call.data == b'' assert call.timeout == 0.0 - assert isinstance(call.callback, collections.Callable) + assert isinstance(call.callback, Callable) def test_single_scp_packet(mock_conn): From ad87af427d5b7eaff7e134f3d987245df0bff81b Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:03:40 +0100 Subject: [PATCH 03/10] Don't pass random.sample a set in random placer The random.sample no longer accepts sets (which to be fair was always a bit of a surprise!). --- rig/place_and_route/place/rand.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rig/place_and_route/place/rand.py b/rig/place_and_route/place/rand.py index dedf2c2..87f627d 100644 --- a/rig/place_and_route/place/rand.py +++ b/rig/place_and_route/place/rand.py @@ -76,7 +76,7 @@ def place(vertices_resources, nets, machine, constraints, movable_vertices = [v for v in vertices_resources if v not in placements] - locations = set(machine) + locations = list(machine) for vertex in movable_vertices: # Keep choosing random chips until we find one where the vertex fits. From 529d7e783223471a429c82c44330a554034b2e68 Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:07:30 +0100 Subject: [PATCH 04/10] Work-around getargspec removal This is something of a temporary fix whilst Python 2.x support remains. --- rig/utils/contexts.py | 17 ++++++++++++++++- rig/utils/docstrings.py | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/rig/utils/contexts.py b/rig/utils/contexts.py index e9bc589..59cc343 100644 --- a/rig/utils/contexts.py +++ b/rig/utils/contexts.py @@ -127,7 +127,22 @@ def f(self, **kwargs): def decorator(f): # Extract any positional and positional-and-key-word arguments # which may be set. - arg_names, varargs, keywords, defaults = inspect.getargspec(f) + if hasattr(inspect, "getfullargspec"): # pragma: no cover + # Python 3 + ( + arg_names, + varargs, + keywords, # Actually varkw + defaults, + # Unused for consistency with Python 2 behaviour, whilst + # Python 2 support remains. + _kwonlyargs, + _kwonlydefaults, + _anotations, + ) = inspect.getfullargspec(f) + else: # pragma: no cover + # Python 2 + arg_names, varargs, keywords, defaults = inspect.getargspec(f) # Sanity check: non-keyword-only arguments should't be present in # the keyword-only-arguments list. diff --git a/rig/utils/docstrings.py b/rig/utils/docstrings.py index 9e45283..f19217e 100644 --- a/rig/utils/docstrings.py +++ b/rig/utils/docstrings.py @@ -109,7 +109,22 @@ def f(*args, kw_only_arg=123) """ def decorate(f_wrapper): - args, varargs, keywords, defaults = inspect.getargspec(f) + if hasattr(inspect, "getfullargspec"): # pragma: no cover + # Python 3 + ( + args, + varargs, + keywords, # Actually varkw + defaults, + # Unused for consistency with Python 2 behaviour, whilst + # Python 2 support remains. + _kwonlyargs, + _kwonlydefaults, + _anotations, + ) = inspect.getfullargspec(f) + else: # pragma: no cover + # Python 2 + args, varargs, keywords, defaults = inspect.getargspec(f) # Simplifies later logic if defaults is None: From e05b51021e15be79887d03c829ca99fde79910b6 Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:08:31 +0100 Subject: [PATCH 05/10] Silence pytest warnings for custom marks --- tests/conftest.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index c116da8..b715878 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -38,6 +38,13 @@ def pytest_addoption(parser): "Specify the IP address or hostname of " "the BMP to use.") +def pytest_configure(config): + config.addinivalue_line("markers", "order_before") + config.addinivalue_line("markers", "order_after") + config.addinivalue_line("markers", "order_id") + config.addinivalue_line("markers", "incremental") + config.addinivalue_line("markers", "no_boot") + # From pytest.org def pytest_runtest_makereport(item, call): # pragma: no cover From 123848752c9c7b9672b9e8bcbd309dfbd1de554d Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:09:05 +0100 Subject: [PATCH 06/10] Remove tests for signal IntEnum types Testing of one IntEnum type value is 'in' another IntEnum type now returns true if the matching integer appears in both. In the past this wasn't the case. Manually making the implementation do the equivalent type checking seems unnecessary and would be better served by a future move to a typed API. To be clear, this change makes the test less strict by no longer checking to see that other IntEnum types are rejected -- since they're not! --- tests/machine_control/test_machine_controller.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/machine_control/test_machine_controller.py b/tests/machine_control/test_machine_controller.py index 6bbabf4..d88ab1e 100644 --- a/tests/machine_control/test_machine_controller.py +++ b/tests/machine_control/test_machine_controller.py @@ -1999,11 +1999,10 @@ def read_struct_field(fn, x, y, p): assert "(0, 1, 4)" in str(excinfo.value) - @pytest.mark.parametrize("signal", ["non-existant", - consts.AppDiagnosticSignal.AND]) + @pytest.mark.parametrize("signal", ["non-existant", -1]) def test_send_signal_fails(self, signal): # Make sure that the send_signal function rejects bad signal - # identifiers (or ones that require special treatment) + # identifiers cn = MachineController("localhost") with pytest.raises(ValueError): cn.send_signal(signal) @@ -2094,11 +2093,10 @@ def test_count_cores_in_state_iterable_of_states(self, states, exp): # Check the correct number of packets were sent assert cn._send_scp.call_count == len(states) - @pytest.mark.parametrize("state", ["non-existant", - consts.AppDiagnosticSignal.AND]) + @pytest.mark.parametrize("state", ["non-existant", -1]) def test_count_cores_in_state_fails(self, state): # Make sure that the count_cores_in_state function rejects bad state - # identifiers (or ones that require special treatment) + # identifiers cn = MachineController("localhost") with pytest.raises(ValueError): cn.count_cores_in_state(state) From 60534c2d5bb38f88767a46c548829161f2627ce8 Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:12:03 +0100 Subject: [PATCH 07/10] Fix broken mock 'assert_called_with' tests The mock library now catches this common mistake and indeed, there are several instances in the test suite which are fixed by this commit. Thanks mock. Thmock. --- tests/machine_control/test_unbooted_ping.py | 4 ++-- tests/scripts/test_rig_discover.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/machine_control/test_unbooted_ping.py b/tests/machine_control/test_unbooted_ping.py index c78d8a2..94dd8a9 100644 --- a/tests/machine_control/test_unbooted_ping.py +++ b/tests/machine_control/test_unbooted_ping.py @@ -24,5 +24,5 @@ def test_listen(should_fail, monkeypatch): assert retval == "127.0.0.1" # Make sure parameters were obayed - assert mock_socket.settimeout.called_once_with(12.0) - assert mock_socket.bind.called_once_with('0.0.0.0', 12345) + mock_socket.settimeout.assert_called_once_with(12.0) + mock_socket.bind.assert_called_once_with(('0.0.0.0', 12345)) diff --git a/tests/scripts/test_rig_discover.py b/tests/scripts/test_rig_discover.py index 753d3f0..8a9f043 100644 --- a/tests/scripts/test_rig_discover.py +++ b/tests/scripts/test_rig_discover.py @@ -24,4 +24,4 @@ def test_rig_discover(args, should_work, timeout, monkeypatch, capsys): else: assert out == "" - assert mock_listen.called_once_with(timeout) + mock_listen.assert_called_once_with(timeout=timeout) From 33c8052689dde93f6df032e18fe44344948d001f Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:14:13 +0100 Subject: [PATCH 08/10] Fix disallowed out-of-range np.int8 to -ve cast This is no longer allowed from a native Python type -- but is from a numpy type! --- rig/type_casts.py | 4 ++-- tests/test_type_casts.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rig/type_casts.py b/rig/type_casts.py index 94165eb..2e28c2c 100644 --- a/rig/type_casts.py +++ b/rig/type_casts.py @@ -380,13 +380,13 @@ class NumpyFixToFloatConverter(object): This will produced signed and unsigned values depending on the `dtype` of the original array. - >>> signed = np.array([0xf0], dtype=np.int8) + >>> signed = np.array(np.array([0xf0]), dtype=np.int8) >>> kbits(signed) array([-1.]) >>> unsigned = np.array([0xf0], dtype=np.uint8) >>> kbits(unsigned)[0] - 15.0 + np.float64(15.0) """ def __init__(self, n_frac): """Create a new converter from fix-point to floating point diff --git a/tests/test_type_casts.py b/tests/test_type_casts.py index bde9733..34b916b 100644 --- a/tests/test_type_casts.py +++ b/tests/test_type_casts.py @@ -245,7 +245,7 @@ class TestNumpyFixToFloat(object): ] ) def test_standard(self, values, dtype, n_frac, expected_values): - input_array = np.array(values, dtype=dtype) + input_array = np.array(np.array(values), dtype=dtype) fpf = NumpyFixToFloatConverter(n_frac) output_array = fpf(input_array) From f2a4237184729b7571e67668b8a38563dcc91b85 Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:16:03 +0100 Subject: [PATCH 09/10] Update pytest syntax for marked test paramters --- tests/place_and_route/place/test_generic_place.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/place_and_route/place/test_generic_place.py b/tests/place_and_route/place/test_generic_place.py index 6a8d251..00eb837 100644 --- a/tests/place_and_route/place/test_generic_place.py +++ b/tests/place_and_route/place/test_generic_place.py @@ -56,8 +56,9 @@ (sa_place, {}), # Test using other kernels (when available) (sa_place, {"kernel": PythonKernel}), - pytest.mark.skipif("CKernel is None")( - (sa_place, {"kernel": CKernel})), + pytest.param(sa_place, + {"kernel": CKernel}, + marks=pytest.mark.skipif("CKernel is None")), # Testing with effort = 0 tests the initial (random) # placement solutions of the SA placer. (sa_place, {"effort": 0.0}), From 1d14fd3058967f1f9993f9ebdae3c0318626fed7 Mon Sep 17 00:00:00 2001 From: Jonathan Heathcote Date: Wed, 7 May 2025 16:16:31 +0100 Subject: [PATCH 10/10] Fix error message checking in test We were checking the ExecInfo rather than the underlying exception. Pytest must have removed the __str__ implementation at some point. --- tests/routing_table/test_routing_table_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/routing_table/test_routing_table_utils.py b/tests/routing_table/test_routing_table_utils.py index aa7c08c..88d1a25 100644 --- a/tests/routing_table/test_routing_table_utils.py +++ b/tests/routing_table/test_routing_table_utils.py @@ -262,9 +262,9 @@ def test_routing_tree_to_tables_repeated_key_mask_fork_not_allowed(): with pytest.raises(MultisourceRouteError) as err: routing_tree_to_tables(routes, net_keys) - assert "(1, 1)" in str(err) # Co-ordinate of the fork - assert "0x00000000" in str(err) # Key that causes the problem - assert "0x0000000f" in str(err) # Mask that causes the problem + assert "(1, 1)" in str(err.value) # Co-ordinate of the fork + assert "0x00000000" in str(err.value) # Key that causes the problem + assert "0x0000000f" in str(err.value) # Mask that causes the problem def test_build_routing_table_target_lengths():