From fbcb674547c8d6912be7e3b06aa2b8bb8a78f13d Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Wed, 9 Oct 2024 12:07:02 +0200 Subject: [PATCH 1/3] improve message for missing keys --- src/qtoolkit/io/base.py | 18 ++++++++++++++++-- tests/io/test_base.py | 14 +++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/qtoolkit/io/base.py b/src/qtoolkit/io/base.py index b935949..c48e627 100644 --- a/src/qtoolkit/io/base.py +++ b/src/qtoolkit/io/base.py @@ -1,6 +1,7 @@ from __future__ import annotations import abc +import difflib import shlex from dataclasses import fields from pathlib import Path @@ -82,9 +83,22 @@ def generate_header(self, options: dict | QResources | None) -> str: # check that all the options are present in the template keys = set(options.keys()) - extra = keys.difference(template.get_identifiers()) + all_identifiers = template.get_identifiers() + extra = keys.difference(all_identifiers) if extra: - msg = f"The following keys are not present in the template: {', '.join(sorted(extra))}" + close_matches = {} + for extra_val in extra: + m = difflib.get_close_matches(extra_val, all_identifiers, n=1) + if m: + close_matches[extra_val] = m[0] + msg = ( + f"The following keys are not present in the template: {', '.join(sorted(extra))}. " + f"Check the template in {type(self).__module__}.{type(self).__qualname__}.header_template" + ) + if close_matches: + msg += "Possible replacements:" + for extra_val, match in close_matches.items(): + msg += f" {match} instead of {extra_val}." raise ValueError(msg) unclean_header = template.safe_substitute(options) diff --git a/tests/io/test_base.py b/tests/io/test_base.py index 09aa6a5..1d50541 100644 --- a/tests/io/test_base.py +++ b/tests/io/test_base.py @@ -146,7 +146,7 @@ def test_generate_header(self, scheduler): with pytest.raises( ValueError, - match=r"The following keys are not present in the template: tata, titi", + match=r"The following keys are not present in the template: tata, titi. Check the template in .*MyScheduler.header_template", ): res = QResources( nodes=4, @@ -155,6 +155,18 @@ def test_generate_header(self, scheduler): ) scheduler.generate_header(res) + with pytest.raises( + ValueError, + match=r"The following keys are not present in the template: option32, processes-per-node. " + r"Check the template in .*MyScheduler.header_template.*option3 instead of option32. processes_per_node instead of processes-per-node", + ): + res = QResources( + nodes=4, + processes_per_node=16, + scheduler_kwargs={"option32": "xxx", "processes-per-node": "yyy"}, + ) + scheduler.generate_header(res) + def test_generate_ids_list(self, scheduler): ids_list = scheduler.generate_ids_list( [QJob(job_id=4), QJob(job_id="job_id_abc1"), 215, "job12345"] From 792f7ee61b11552e106b6f97b012df49648589c5 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Wed, 9 Oct 2024 13:38:17 +0200 Subject: [PATCH 2/3] fix msg order for reproducibility --- src/qtoolkit/io/base.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qtoolkit/io/base.py b/src/qtoolkit/io/base.py index c48e627..bff6e3a 100644 --- a/src/qtoolkit/io/base.py +++ b/src/qtoolkit/io/base.py @@ -93,12 +93,12 @@ def generate_header(self, options: dict | QResources | None) -> str: close_matches[extra_val] = m[0] msg = ( f"The following keys are not present in the template: {', '.join(sorted(extra))}. " - f"Check the template in {type(self).__module__}.{type(self).__qualname__}.header_template" + f"Check the template in {type(self).__module__}.{type(self).__qualname__}.header_template." ) if close_matches: - msg += "Possible replacements:" - for extra_val, match in close_matches.items(): - msg += f" {match} instead of {extra_val}." + msg += " Possible replacements:" + for extra_val in sorted(close_matches): + msg += f" {close_matches[extra_val]} instead of {extra_val}." raise ValueError(msg) unclean_header = template.safe_substitute(options) From 8ef8beb921e5f127d60520a38d21a6bad8a82679 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Wed, 9 Oct 2024 15:00:56 +0200 Subject: [PATCH 3/3] improve error message --- src/qtoolkit/io/base.py | 11 ++++++++--- tests/io/test_base.py | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/qtoolkit/io/base.py b/src/qtoolkit/io/base.py index bff6e3a..833d8ac 100644 --- a/src/qtoolkit/io/base.py +++ b/src/qtoolkit/io/base.py @@ -88,9 +88,11 @@ def generate_header(self, options: dict | QResources | None) -> str: if extra: close_matches = {} for extra_val in extra: - m = difflib.get_close_matches(extra_val, all_identifiers, n=1) + m = difflib.get_close_matches( + extra_val, all_identifiers, n=3, cutoff=0.65 + ) if m: - close_matches[extra_val] = m[0] + close_matches[extra_val] = m msg = ( f"The following keys are not present in the template: {', '.join(sorted(extra))}. " f"Check the template in {type(self).__module__}.{type(self).__qualname__}.header_template." @@ -98,7 +100,10 @@ def generate_header(self, options: dict | QResources | None) -> str: if close_matches: msg += " Possible replacements:" for extra_val in sorted(close_matches): - msg += f" {close_matches[extra_val]} instead of {extra_val}." + replacements = " or ".join( + f"'{m}'" for m in close_matches[extra_val] + ) + msg += f" {replacements} instead of '{extra_val}'." raise ValueError(msg) unclean_header = template.safe_substitute(options) diff --git a/tests/io/test_base.py b/tests/io/test_base.py index 1d50541..bc05ad5 100644 --- a/tests/io/test_base.py +++ b/tests/io/test_base.py @@ -144,9 +144,12 @@ def test_generate_header(self, scheduler): #SPECCMD --nodes=4""" ) + # check that the error message contains the expected error, but should not match + # the possible replacements, as they are too different with pytest.raises( ValueError, - match=r"The following keys are not present in the template: tata, titi. Check the template in .*MyScheduler.header_template", + match=r"The following keys are not present in the template: tata, titi. Check " + r"the template in .*MyScheduler.header_template(?!.*instead of 'titi')", ): res = QResources( nodes=4, @@ -158,7 +161,8 @@ def test_generate_header(self, scheduler): with pytest.raises( ValueError, match=r"The following keys are not present in the template: option32, processes-per-node. " - r"Check the template in .*MyScheduler.header_template.*option3 instead of option32. processes_per_node instead of processes-per-node", + r"Check the template in .*MyScheduler.header_template.*'option3' or 'option2' or 'option1' " + r"instead of 'option32'. 'processes_per_node' or 'processes' instead of 'processes-per-node'", ): res = QResources( nodes=4,