-
Notifications
You must be signed in to change notification settings - Fork 22
Add option to partition results table by measurements #198
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
base: main
Are you sure you want to change the base?
Changes from 7 commits
4f57b1e
f9f2f91
49bdd44
cd89838
8c909ea
a7e12e7
c71f051
c8d822d
8a5d818
97ae7fd
9d779ec
2bb8bcf
7d707cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| "scripts": { | ||
| "prepare": "if [ -f './tsconfig.json' ]; then npm run build; fi;", | ||
| "build": "rimraf lib/ client/lib/ && mkdir lib && npm run generate-json-schema && tsc && tsc -p client/ && npm run lint", | ||
| "build:watch": "tsc --watch", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call! |
||
| "generate-json-schema": "typescript-json-schema tsconfig.json ConfigFile --include src/configfile.ts --required --noExtraProps > config.schema.json", | ||
| "lint": "tslint --project . --format stylish", | ||
| "format": "clang-format --style=file -i \"--glob=./{client,}/src/**/*.ts\"", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ import ansi = require('ansi-escape-sequences'); | |
|
|
||
| import {Difference, ConfidenceInterval, ResultStats, ResultStatsWithDifferences} from './stats'; | ||
| import {BenchmarkSpec, BenchmarkResult} from './types'; | ||
| import {measurementName} from './measure'; | ||
|
|
||
| export const spinner = ['⠋', '⠙', '⠹', '⠸', '⠼', '⠴', '⠦', '⠧', '⠇', '⠏'].map( | ||
| (frame) => ansi.format(`[blue]{${frame}}`)); | ||
|
|
@@ -104,6 +105,23 @@ export function automaticResultTable(results: ResultStats[]): AutomaticResults { | |
| return {fixed: fixedTable, unfixed: unfixedTable}; | ||
| } | ||
|
|
||
| export function collatedResultTables(results: ResultStatsWithDifferences[]) { | ||
|
||
| const collated: {[index: string]: ResultStatsWithDifferences[]} = {}; | ||
| results.forEach((result) => { | ||
| const meas = measurementName(result.result.measurement); | ||
| (collated[meas] || (collated[meas] = [])).push({ | ||
| ...result, | ||
| differences: result.differences.filter( | ||
| (_, i) => measurementName(results[i].result.measurement) === meas) | ||
| }); | ||
| }); | ||
| const tables: AutomaticResults[] = []; | ||
| for (const results of Object.values(collated)) { | ||
| tables.push(automaticResultTable(results)); | ||
| } | ||
| return tables; | ||
| } | ||
|
|
||
| /** | ||
| * Format a terminal text result table where each result is a row: | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
partition: "measurement"?I also thought about names
compareandgroup-by, but I'm leaning towardspartitionbecause it seems to best describe "the thing you want to do to the table after you see that it's too big".Also I'm thinking the value should be
measurementinstead oftrueso that we have more room to expand with more partitioning options in the future.WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's nice. So conceptually we could add something like
partition: "browser"to partition the results table based on browser? And next level, make it an array to partition on multiple dimensions, i.e. browser and measurement?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Was out yesterday but will try and get an update up soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've addressed the feedback. All instances of
collate: truehave been replaced bypartition: "measurement".I also added documentation to the README for the new option.