Skip to content

Fix pvalue display when --pvalue is not being passed.#492

Merged
eightbitraptor merged 1 commit intomainfrom
mvh-fix-pvalue-display
Mar 17, 2026
Merged

Fix pvalue display when --pvalue is not being passed.#492
eightbitraptor merged 1 commit intomainfrom
mvh-fix-pvalue-display

Conversation

@eightbitraptor
Copy link
Contributor

The p-value indicators should only be included if --pvalue is passed to the run_benchmarks.rb script

The p-value indicators should only be included if --pvalue is passed to
the run_benchmarks.rb script
@eightbitraptor eightbitraptor merged commit dd12edb into main Mar 17, 2026
11 checks passed
@eightbitraptor eightbitraptor deleted the mvh-fix-pvalue-display branch March 17, 2026 17:16
k0kubun added a commit to k0kubun/ruby-bench that referenced this pull request Mar 25, 2026
The ljust padding in format_ratio added trailing spaces to reserve room
for significance symbols, but these spaces were then stripped by rstrip
in the table formatter's format_row, causing the last column to lose its
right-alignment.

This was exposed by ruby#492 which made pval nil when --pvalue is not passed,
meaning the significance symbol is always empty in the common case, yet
the ljust padding was still applied and then stripped.

Remove the ljust padding entirely and let the table formatter handle
column alignment, restoring the original format_ratio logic.
k0kubun added a commit that referenced this pull request Mar 25, 2026
The ljust padding in format_ratio added trailing spaces to reserve room
for significance symbols, but these spaces were then stripped by rstrip
in the table formatter's format_row, causing the last column to lose its
right-alignment.

This was exposed by #492 which made pval nil when --pvalue is not passed,
meaning the significance symbol is always empty in the common case, yet
the ljust padding was still applied and then stripped.

Remove the ljust padding entirely and let the table formatter handle
column alignment, restoring the original format_ratio logic.
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