Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 29 additions & 14 deletions src/workflow/CommandExecutor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
Expand Down
66 changes: 65 additions & 1 deletion src/workflow/ParameterManager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import shutil
import subprocess
import streamlit as st
import defusedxml.ElementTree as ET
from pathlib import Path

class ParameterManager:
Expand Down Expand Up @@ -287,4 +288,67 @@ 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]
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 <NODE name="ToolName">.
# 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:
logger.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 []