Conversation
streeve
left a comment
There was a problem hiding this comment.
So this can be either the list or the individual step now?
I added an example in the PR summary to illustrate the purpose of this better. |
|
Sorted out the CI failures. Part of the previous CI failures was actually catching a bug in the code. The other part of it was due to oversubscribing during the AdditiveFOAM examples using |
|
@streeve, I cleaned this up. It should be good to go now. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the argument list assembly to support list-valued arguments and improve code structure. The introduction of the _get_arglist helper function successfully centralizes argument parsing logic and removes code duplication. However, there is a subtle bug in the implementation of _get_arglist due to the way boolean types are handled in Python, which could lead to incorrect command-line arguments for False values. I've provided a specific comment with a code suggestion to fix this. Overall, this is a good refactoring that improves functionality and maintainability.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Cherry-picked feature from #133.
This simplifies the configure, execute, and postprocess argument handling and adds capability needed to pass list-values as arguments to these steps.
Example:
Myna input file:
Becomes this argument list for the configure step, which is correctly handled by argparse, assuming the
--nvaluesparameter is configured to take multiple entries:The previous implementation would have returned the following, which argparse can't handle: