Add basic handling for flags

[poshul]

PR Type

enhancement

---

Description

• Improved handling of command line flags in CommandExecutor.py:
  • Flags without a true value are now correctly omitted from the command.
  • Only the flag name is added for boolean true values, avoiding incorrect command line arguments.

---

Changes walkthrough 📝

	Relevant files
Enhancement	CommandExecutor.py Enhance Flag Handling in Command Construction                        src/workflow/CommandExecutor.py Enhanced command construction logic to handle flags more appropriately. Flags are now checked for their boolean value, and only the flag name is added if the value is true. Skips appending the value for boolean flags to ensure correct command formation. +8/-2

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Description updated to latest commit (34503ac)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the changes are localized to a specific functionality in the code, and the logic is straightforward. The PR modifies the command line flag handling in a Python script, which is a common task and should be relatively easy to review for an experienced developer.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The condition if v != False: might not behave as expected if v is None or an empty string, which are also falsy values in Python. This could lead to unintended flags being added to the command.
	Code Clarity: The comment # add the value if we aren't a flag might be misleading because it's placed before a block that handles multiline strings, not specifically non-flag values.
🔒 Security concerns	No

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestions
Best practice	Use identity comparison for boolean checks. Replace the check if v != False with if v is not False to ensure that the comparison is done using identity rather than equality. This is more Pythonic and avoids potential issues with different types that may evaluate to false. src/workflow/CommandExecutor.py [170] -if v != False: +if v is not False:
Enhancement	Add a check for None values to prevent errors. Consider adding a check for None values before appending to the command list to avoid potential errors when None is inadvertently passed as a value. src/workflow/CommandExecutor.py [179] -command += [str(v)] +if v is not None: + command += [str(v)]
	Add error handling for file removal operations. Ensure that the temporary file is safely removed by using a try-except block to handle potential exceptions during the unlink operation. src/workflow/CommandExecutor.py [274] -tmp_params_file.unlink() +try: + tmp_params_file.unlink() +except Exception as e: + print(f"Failed to unlink file: {e}")
Maintainability	Clarify the condition for handling boolean flags. Use a more explicit condition check for boolean flags by checking if isinstance(v, bool) and v: instead of just elif isinstance(v, bool):. This ensures that the logic is clear and only true boolean flags are considered. src/workflow/CommandExecutor.py [176] -elif isinstance(v, bool): +elif isinstance(v, bool) and v:
	Refactor command construction into a separate method. Refactor the command construction logic into a separate method to improve readability and maintainability of the run_topp method. src/workflow/CommandExecutor.py [167-179] -if tool in params.keys(): - for k, v in params[tool].items(): +def construct_command(params): + command = [] + for k, v in params.items(): if v is not False: - command += [f"-{k}"] + command.append(f"-{k}") if isinstance(v, str) and "\n" in v: - command += v.split("\n") + command.extend(v.split("\n")) elif isinstance(v, bool) and v: continue else: - command += [str(v)] + command.append(str(v)) + return command

[axelwalter]

Running this with the example workflow results in errors for non-flag boolean values, which require "true" or "false" passed as string.

Invalid string parameter value '' for parameter 'enable_RT_filtering' given! Valid values are: 'false,true'. Updating failed!

[axelwalter]

Setting the value depending on bool or string type might solve it?

streamlit-template/src/workflow/StreamlitUI.py

Line 420 in bbbd104

"value": entry.value,

[poshul]

Running this with the example workflow results in errors for non-flag boolean values, which require "true" or "false" passed as string.

Invalid string parameter value '' for parameter 'enable_RT_filtering' given! Valid values are: 'false,true'. Updating failed!

Hmm, thats a problem. Underlying this is gonna be the fact that since ParamValue doesn't have a type for Bool (https://github.com/OpenMS/OpenMS/blob/develop/src/openms/include/OpenMS/DATASTRUCTURES/ParamValue.h#L37-L46) and the "type" field from the ini file doesn't get stored, that I don't think we can tell the difference between a flag and a string option that just has true or false as its options from the option itself.

Maybe we can check the allowable values as well to distinguish?

[cbielow]

Running this with the example workflow results in errors for non-flag boolean values, which require "true" or "false" passed as string.
Invalid string parameter value '' for parameter 'enable_RT_filtering' given! Valid values are: 'false,true'. Updating failed!

Hmm, thats a problem. Underlying this is gonna be the fact that since ParamValue doesn't have a type for Bool (https://github.com/OpenMS/OpenMS/blob/develop/src/openms/include/OpenMS/DATASTRUCTURES/ParamValue.h#L37-L46) and the "type" field from the ini file doesn't get stored, that I don't think we can tell the difference between a flag and a string option that just has true or false as its options from the option itself.

Maybe we can check the allowable values as well to distinguish?

use ParamValue::toBool()