Fix longitudinal plots with a single comparison version. #421
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… single subplot, pyplot.subplots's second argument is not an array.
mdboom
approved these changes
Apr 23, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where a single subplot causes errors due to plt.subplots returning a single Axes instead of a list, ensuring consistent handling of axes for longitudinal plots.
- Introduces a variable (numvers) to store the number of versions from the configuration
- Updates figure size calculations and wraps the single Axes into a list for consistent iteration
Comments suppressed due to low confidence (1)
bench_runner/plot.py:263
- [nitpick] Consider renaming 'numvers' to 'num_versions' for improved clarity.
numvers = len(cfg["versions"])
mdboom
added a commit
to mdboom/bench_runner
that referenced
this pull request
Apr 24, 2025
This removes all of the hardcoding of corner-cases in the plotting code. Fixes faster-cpython#424. Supercedes faster-cpython#423 and faster-cpython#421.
Merged
|
Superceded by #425. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If there's a single subplot, pyplot.subplots's second return value is not an array, so we can't zip() it.
There is at least one hard-coded assumption about the longitudinal plots (specifically, that the third plot is for JIT) but while that's probably a bit confusing if you'd want to have three plots for different versions, at least it appears clearly in the output label. (I've been thinking about ways to make the plotting more configurable, but in the mean time this fixes a bug when you only care about one longitudinal plot.)