Remove double setting of default values

[juanmichelini]

Default values should be read at args setting, not function calls

So we should remove EVAL_DEFAULTS from

def run_swebench_evaluation(
    predictions_file: str,
    run_id: str,
    dataset: str = EVAL_DEFAULTS["dataset"],
    workers: int = EVAL_DEFAULTS["workers"],
    split: str = EVAL_DEFAULTS["split"],
    modal: bool = EVAL_DEFAULTS["modal"],
    timeout: int = EVAL_DEFAULTS["timeout"],

[juanmichelini]

@OpenHands please create a PR for this, check other benchmarks, and justify if the change requires testing or not.

[openhands-ai]

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

[openhands-ai]

Summary

I've successfully created PR #399 to fix issue #398: #399

Changes Made

benchmarks/swebench/eval_infer.py:

• Removed EVAL_DEFAULTS from run_swebench_evaluation() function signature (5 parameters: dataset, workers, split, modal, timeout)

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

I checked other benchmarks in the repository and confirmed this change aligns with the established pattern:

• 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 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 are only called from their respective main() functions, which always pass explicit arguments from the parsed args.

3. 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 (and the commit passed all pre-commit checks).

Checklist

• ✅ Created branch with openhands/ prefix
• ✅ Committed changes with clear message
• ✅ Pushed branch to GitHub
• ✅ Created PR that fixes/closes issue Remove double setting of default values #398
• ✅ PR description includes justification for no new tests

View full conversation