From 45ae41d461294e832042e288ba656f7584466673 Mon Sep 17 00:00:00 2001 From: Nick Moore Date: Tue, 3 Sep 2024 22:09:35 +1000 Subject: [PATCH] fix 'None' string parameters, plus code tidy --- countess/core/cmd.py | 34 ++++++++++++++++++---------------- countess/core/parameters.py | 6 +++--- countess/core/pipeline.py | 17 ++++++++++++----- countess/gui/config.py | 6 ++++-- countess/plugins/data_table.py | 5 +++++ countess/plugins/python.py | 2 +- tests/test_config.py | 3 ++- tests/test_pipeline.py | 8 +++----- 8 files changed, 48 insertions(+), 33 deletions(-) diff --git a/countess/core/cmd.py b/countess/core/cmd.py index 3ff1ea1..88d36bd 100644 --- a/countess/core/cmd.py +++ b/countess/core/cmd.py @@ -6,29 +6,27 @@ import re import sys - from countess import VERSION from countess.core.config import read_config logger = logging.getLogger(__name__) -def run(args : list[str]) -> None: - options, args = getopt.getopt(args, '', ['version', 'set=', 'log=']) - config : list[tuple[str,str,str]] = [] - +def run(args: list[str]) -> None: + options, args = getopt.getopt(args, "", ["version", "set=", "log="]) + config: list[tuple[str, str, str]] = [] for opt_key, opt_val in options: - if opt_key == '--version': + if opt_key == "--version": print(f"CountESS {VERSION}") # pylint: disable=bad-builtin - elif opt_key == '--set': - if m := re.match(r'([^.]+)\.([^=]+)=(.*)', opt_val): - config.append(m.groups()) + elif opt_key == "--set": + if m := re.match(r"([^.]+)\.([^=]+)=(.*)", opt_val): + config.append((m.group(1), m.group(2), m.group(3))) else: logger.warning("Bad --set option: %s", opt_val) - elif opt_key == '--log': + elif opt_key == "--log": try: - if re.match(r'\d+$', opt_val): + if re.match(r"\d+$", opt_val): logging.getLogger().setLevel(int(opt_val)) else: logging.getLogger().setLevel(opt_val.upper()) @@ -39,11 +37,15 @@ def run(args : list[str]) -> None: for filename in args: graph = read_config(filename) - for node_label, config_key, config_val in config: - try: - graph.find_node(node_label).set_config(config_key, ast.literal_eval(config_val), '.') - except (KeyError, ValueError): - logger.warning("Bad --set option: %s=%s", config_key, config_val) + for node_name, config_key, config_val in config: + node = graph.find_node(node_name) + if node: + try: + node.set_config(config_key, ast.literal_eval(config_val), ".") + except (TypeError, KeyError, ValueError): + logger.warning("Bad --set option: %s.%s=%s", node_name, config_key, config_val) + else: + logger.warning("Bad --set node name: %s", node_name) graph.run() diff --git a/countess/core/parameters.py b/countess/core/parameters.py index 3f515c2..ee1081a 100644 --- a/countess/core/parameters.py +++ b/countess/core/parameters.py @@ -130,7 +130,7 @@ class StringParam(ScalarWithOperatorsParam): _value: Optional[str] = None def set_value(self, value: Any): - self._value = str(value) + self._value = str(value or "") # Operator methods which apply only to strings @@ -149,7 +149,7 @@ class TextParam(StringParam): long text field and also removes extra blank lines""" def set_value(self, value): - self._value = re.sub("\n\n\n+", "\n\n", value) + self._value = re.sub("\n\n\n+", "\n\n", value or "") class NumericParam(ScalarWithOperatorsParam): @@ -270,7 +270,7 @@ def clean_character(self, c: str): return "" def set_value(self, value: Any): - value_str = str(value) + value_str = str(value or "") self._value = "".join([self.clean_character(c) for c in value_str]) def copy(self) -> "StringCharacterSetParam": diff --git a/countess/core/pipeline.py b/countess/core/pipeline.py index 432e5f4..c895814 100644 --- a/countess/core/pipeline.py +++ b/countess/core/pipeline.py @@ -79,7 +79,15 @@ class PipelineNode: # at config load time, if it is present it is loaded the # first time the plugin is prerun. - def __init__(self, name, plugin=None, position=None, notes=None, sort_column=0, sort_descending=0): + def __init__( + self, + name: str, + plugin: Optional[BasePlugin] = None, + position: Optional[tuple[float, float]] = None, + notes: Optional[str] = None, + sort_column: int = 0, + sort_descending: bool = False, + ): self.name = name self.plugin = plugin self.position = position or (0.5, 0.5) @@ -89,7 +97,7 @@ def __init__(self, name, plugin=None, position=None, notes=None, sort_column=0, self.parent_nodes = set() self.child_nodes = set() self.output_queues = set() - self.config : list[tuple[str,str,str]] = [] + self.config: list[tuple[str, str, str]] = [] def set_config(self, key, value, base_dir): self.config.append((key, value, base_dir)) @@ -270,9 +278,9 @@ def del_node(self, node: PipelineNode): node.detach() self.nodes.remove(node) - def find_node(self, label: str) -> Optional[PipelineNode]: + def find_node(self, name: str) -> Optional[PipelineNode]: for node in self.nodes: - if node.label == label: + if node.name == name: return node return None @@ -329,7 +337,6 @@ def reset_node_names(self): node.name += f" {num + 1}" node_names_seen.add(node.name) - def tidy(self): """Tidies the graph (sets all the node positions)""" diff --git a/countess/gui/config.py b/countess/gui/config.py index 8dfb65a..6205d18 100644 --- a/countess/gui/config.py +++ b/countess/gui/config.py @@ -208,7 +208,10 @@ def update(self): elif isinstance(self.parameter, FileParam): self.entry["text"] = self.parameter.value else: - self.var.set(self.parameter.value) + if self.parameter.value is None: + self.var.set("") + else: + self.var.set(self.parameter.value) if self.label: self.label["text"] = self.parameter.label @@ -448,7 +451,6 @@ def change_parameter(self, parameter): self.change_callback(self) def update(self) -> None: - # If there's only a single parameter it is presented a little differently. top_level = 0 if len(self.plugin.params) == 1 else 1 diff --git a/countess/plugins/data_table.py b/countess/plugins/data_table.py index 328b24f..90c3cd5 100644 --- a/countess/plugins/data_table.py +++ b/countess/plugins/data_table.py @@ -36,6 +36,11 @@ class DataTablePlugin(PandasInputPlugin): def fix_columns(self): old_rows = self.rows.params + # XXX should deal with duplicate column names more generally + for num, col in enumerate(self.columns): + if col.name == "": + col.name = f"col_{num+1}" + self.params["rows"] = self.rows = ArrayParam( "Rows", TabularMultiParam("Row", {str(col.name): col.type.get_parameter(str(col.name)) for col in self.columns}), diff --git a/countess/plugins/python.py b/countess/plugins/python.py index bd76976..0da17bd 100644 --- a/countess/plugins/python.py +++ b/countess/plugins/python.py @@ -100,7 +100,7 @@ def process_dataframe(self, dataframe: pd.DataFrame) -> pd.DataFrame: b) we don't need to merge afterwards""" # XXX cache this? - self.code_object = compile(self.code.value, "", mode="exec") + self.code_object = compile(self.code.value or "", "", mode="exec") dataframe = dataframe.reset_index(drop=dataframe.index.names == [None]) series = self.dataframe_to_series(dataframe) diff --git a/tests/test_config.py b/tests/test_config.py index e27216b..0971425 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -26,7 +26,8 @@ def test_read_config_dict_no_plugin(): def test_write_config(): - pn = PipelineNode("node", plugin=NothingPlugin("node"), config=[("foo", "bar", "baz")]) + pn = PipelineNode("node", plugin=NothingPlugin("node")) + pn.set_config("foo", "bar", "baz") pg = PipelineGraph([pn]) buf = io.StringIO() diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 4a1e09e..e1124aa 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -106,12 +106,10 @@ def test_plugin_config(caplog): dnp = DoesNothingPlugin() dnn = PipelineNode( "node", - plugin=dnp, - config=[ - ("param", 1, "."), - ("noparam", "whatever", "."), - ], + plugin=dnp ) + dnn.set_config("param", "1", ".") + dnn.set_config("noparam", "whatever", ".") dnn.load_config() assert "noparam=whatever" in caplog.text