Skip to content

Fix #377: Provide a stable ordering for results #381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 26, 2025
Merged

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Mar 26, 2025

Cc: @Yhg1s

@mdboom mdboom requested a review from Copilot March 26, 2025 15:53
Copy link

@Copilot 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 aims to fix ordering inconsistencies by ensuring that result items always follow a stable order. The changes introduce a tuple conversion for flags and filename as a tie-breaker in sorting, and update filename parsing in plotting.

  • In bench_runner/result.py, tuple(x.flags) and x.filename have been added to the sorting key.
  • In bench_runner/plot.py, the functions retrieving results have been updated to use from_arbitrary_filename.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
bench_runner/result.py Adds tuple conversion of flags and filename to enforce stable order
bench_runner/plot.py Updates result retrieval to use from_arbitrary_filename
Comments suppressed due to low confidence (1)

bench_runner/plot.py:488

  • Ensure that the new function 'from_arbitrary_filename' is covered by tests to verify that it handles all expected filename scenarios consistently with previous behavior.
ref = result.Result.from_arbitrary_filename(Path(args.ref))

@mdboom mdboom force-pushed the stable-ordering branch 2 times, most recently from 99e7495 to a0048ac Compare March 26, 2025 17:09
@mdboom mdboom merged commit f1cb96a into main Mar 26, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant