Remove double setting of default values in function signatures

[juanmichelini]

Summary

This PR removes the double setting of default values in function signatures. Default values should be read at args setting via parser.set_defaults(), not in function call signatures.

Fixes #398

Changes

benchmarks/swebench/eval_infer.py

• Removed EVAL_DEFAULTS from run_swebench_evaluation() function signature parameters

benchmarks/swtbench/eval_infer.py

• Removed EVAL_DEFAULTS["split"] from try_pull_prebaked_images() function signature
• Updated the call site to explicitly pass args.split

Consistency with Other Benchmarks

This change aligns with the pattern used by other benchmarks in the repository:

• swebenchmultimodal/eval_infer.py - uses only parser.set_defaults(**EVAL_DEFAULTS)
• gaia/run_infer.py - uses only parser.set_defaults(**INFER_DEFAULTS)
• commit0/run_infer.py - uses only parser.set_defaults(**INFER_DEFAULTS)
• swtbench/run_infer.py - uses only parser.set_defaults(**INFER_DEFAULTS)

Testing Justification

No new tests are required for this change because:

1. Pure refactoring: This is a code cleanup that doesn't change behavior. The defaults are still applied via parser.set_defaults(**EVAL_DEFAULTS) in the main() function.

2. No external callers: The affected functions (run_swebench_evaluation and try_pull_prebaked_images) are only called from their respective main() functions, which always pass explicit arguments from the parsed args.

3. Existing tests pass: All 32 existing tests pass, confirming the refactoring doesn't break any functionality.

4. Static type checking: The pre-commit hooks include Pyright type checking which validates that all required arguments are passed correctly.

@juanmichelini can click here to continue refining the PR

[simonrosenberg]

@OpenHands are there any other default values that, similarly to this PR, are set twice in the benchmarks repo? If so, make another PR from main and remove them

[openhands-ai]

I'm on it! simonrosenberg can track my progress at all-hands.dev

[openhands-ai]

Yes, I found one additional case of a default value being set twice in the benchmarks repo, similar to PR #399.

Finding:
In benchmarks/swebench/eval_infer.py, the timeout argument had its default set twice:

1. In the add_argument() call with default=EVAL_DEFAULTS["timeout"] (line 264)
2. In parser.set_defaults(**EVAL_DEFAULTS) (line 269)

Action Taken:
I created a new PR from main to fix this issue:

• PR Remove double setting of timeout default in swebench eval_infer.py #417: Remove double setting of timeout default in swebench eval_infer.py #417

Changes made:

• Removed default=EVAL_DEFAULTS["timeout"] from the --timeout argument definition
• Updated the help text to remove the redundant default value display
• Updated the comment to reflect that timeout is now included in the defaults applied by set_defaults()

All 35 tests pass with this change.

View full conversation