Skip to content

Fail a trial if it produces empty csv file#1006

Merged
motus merged 4 commits intomicrosoft:mainfrom
motus:sergiym/storage/null_csv
Oct 23, 2025
Merged

Fail a trial if it produces empty csv file#1006
motus merged 4 commits intomicrosoft:mainfrom
motus:sergiym/storage/null_csv

Conversation

@motus
Copy link
Copy Markdown
Member

@motus motus commented Oct 22, 2025

Pull Request

Title

Fail a trial if it produces empty csv file.
A follow-up to #998


Description

  • Fail a trial if it produces empty csv file.
  • Add unit test to check for the condition.
  • Small fix in split_cmdline() function to allow empty token strings (occurs in the new unit test)

Type of Change

  • 🛠️ Bug fix
  • 🧪 Tests

Testing

Run unit tests as usual

@motus motus self-assigned this Oct 22, 2025
Copilot AI review requested due to automatic review settings October 22, 2025 23:07
@motus motus requested a review from a team as a code owner October 22, 2025 23:07
@motus motus added bug Something isn't working ready for review Ready for review tests Add or fix unit tests mlos-bench labels Oct 22, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances error handling for CSV result files by detecting and failing trials that produce empty CSV files, following up on issue #998. The changes prevent crashes when encountering malformed or empty result files.

Key Changes:

  • Added exception handling for empty CSV files that would otherwise cause pandas to raise an EmptyDataError
  • Fixed a potential IndexError in command line parsing when encountering empty tokens
  • Added comprehensive test coverage for the new empty file scenario

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
mlos_bench/mlos_bench/environments/local/local_env.py Added try-except block to catch EmptyDataError when reading CSV files and fail the trial gracefully
mlos_bench/mlos_bench/services/local/local_exec.py Fixed indexing bug that could cause IndexError with empty tokens by using slice notation
mlos_bench/mlos_bench/tests/environments/local/local_env_test.py Added new test case for zero-length CSV files and clarified existing test for CSV files with headers but no data

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@bpkroth
Copy link
Copy Markdown
Contributor

bpkroth commented Oct 23, 2025

Nice!

@motus motus merged commit 0152cb6 into microsoft:main Oct 23, 2025
16 checks passed
@motus motus deleted the sergiym/storage/null_csv branch October 23, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working mlos-bench ready for review Ready for review tests Add or fix unit tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants