diff --git a/requirements.txt b/requirements.txt index 938ccc1e..95e72be5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,6 +26,7 @@ contourpy==1.3.1 # via matplotlib cycler==0.12.1 # via matplotlib +defusedxml==0.7.1 fonttools==4.59.2 # via matplotlib gitdb==4.0.12 diff --git a/src/workflow/CommandExecutor.py b/src/workflow/CommandExecutor.py index 86f265b2..838173b5 100644 --- a/src/workflow/CommandExecutor.py +++ b/src/workflow/CommandExecutor.py @@ -263,6 +263,10 @@ def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> b # Load parameters for non-defaults params = self.parameter_manager.get_parameters_from_json() + + # NEW LOGIC: Ask the ParameterManager which keys are strictly booleans + bool_params = self.parameter_manager.get_boolean_parameters(tool) + # Construct commands for each process for i in range(n_processes): command = [tool] @@ -281,27 +285,38 @@ def run_topp(self, tool: str, input_output: dict, custom_params: dict = {}) -> b # standard case, files was a list of strings, take the file name at index else: command += [value[i]] + # Add non-default TOPP tool parameters if tool in params.keys(): for k, v in params[tool].items(): + # NEW LOGIC: Intercept boolean flags + if k in bool_params: + # Only add the implicit flag if True. If False, omit entirely. + if str(v).lower() == "true": + command += [f"-{k}"] + else: + # Existing logic for strings, ints, floats + command += [f"-{k}"] + if v != "" and v is not None: + if isinstance(v, str) and "\n" in v: + command += v.split("\n") + else: + command += [str(v)] + + # Add custom parameters + for k, v in custom_params.items(): + # NEW LOGIC: Intercept custom boolean flags + if k in bool_params: + if str(v).lower() == "true": + command += [f"-{k}"] + else: command += [f"-{k}"] - # Skip only empty strings (pass flag with no value) - # Note: 0 and 0.0 are valid values, so use explicit check if v != "" and v is not None: - if isinstance(v, str) and "\n" in v: - command += v.split("\n") + if isinstance(v, list): + command += [str(x) for x in v] else: command += [str(v)] - # Add custom parameters - for k, v in custom_params.items(): - command += [f"-{k}"] - # Skip only empty strings (pass flag with no value) - # Note: 0 and 0.0 are valid values, so use explicit check - if v != "" and v is not None: - if isinstance(v, list): - command += [str(x) for x in v] - else: - command += [str(v)] + # Add threads parameter for TOPP tools command += ["-threads", str(threads_per_command)] commands.append(command) diff --git a/src/workflow/ParameterManager.py b/src/workflow/ParameterManager.py index b0c36263..2aa7e3ac 100644 --- a/src/workflow/ParameterManager.py +++ b/src/workflow/ParameterManager.py @@ -3,6 +3,7 @@ import shutil import subprocess import streamlit as st +import defusedxml.ElementTree as ET from pathlib import Path class ParameterManager: @@ -287,4 +288,68 @@ def clear_parameter_session_state(self) -> None: if key.startswith(self.param_prefix) or key.startswith(self.topp_param_prefix) ] for key in keys_to_delete: - del st.session_state[key] \ No newline at end of file + del st.session_state[key] + + def get_boolean_parameters(self, tool: str) -> list: + """ + Parses the tool's generated .ini (XML) file to discover strictly boolean parameters. + This prevents implicit booleans from being passed as strings to the command line. + + Args: + tool (str): The name of the TOPP tool (e.g., 'FeatureFinderMetabo'). + + Returns: + list: A list of hierarchical parameter keys that are explicitly typed as 'bool'. + + Raises: + FileNotFoundError: If the .ini file does not exist. + RuntimeError: If the .ini file fails to generate. + ET.ParseError: If the XML parsing fails. + """ + if not self.create_ini(tool): + # CodeRabbit Fix: Raise an explicit error instead of silently returning [] + raise RuntimeError(f"Failed to generate .ini file for TOPP tool: {tool}") + + ini_path = Path(self.ini_dir, f"{tool}.ini") + if not ini_path.exists(): + # CodeRabbit Fix: Raise an explicit error instead of silently returning [] + raise FileNotFoundError(f"Missing expected .ini file for TOPP tool at: {ini_path}") + + bool_params = [] + try: + # CodeRabbit Fix: Use defusedxml (imported as ET) to safely parse the file + tree = ET.parse(ini_path) + root = tree.getroot() + + # Recursive function to build the hierarchical path from XML nodes + def traverse(node, current_path): + for child in node: + if child.tag == "ITEM" and child.get("type") == "bool": + name = child.get("name") + if name: + bool_params.append(current_path + name) + elif child.tag == "NODE": + name = child.get("name") + if name: + # Append the node name and a colon to the path + traverse(child, current_path + name + ":") + + # Start traversal from the XML root + traverse(root, "") + + # OpenMS INI files usually encapsulate everything in a top-level . + # We must strip this prefix so the keys perfectly match the JSON session state keys. + tool_prefix = f"{tool}:1:" + cleaned_params = [ + p[len(tool_prefix):] if p.startswith(tool_prefix) else p + for p in bool_params + ] + + return cleaned_params + + except ET.ParseError as e: + st.error(f"XML parsing failed for {tool}: {e}") + raise + print(f"Error parsing boolean parameters for {tool}: {e}") + pass # Safely return empty list if XML parsing fails + return [] \ No newline at end of file