-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-9201: [Archery] More user-friendly console output for benchmark diffs, add repetitions argument, don't build unit tests #7516
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
Conversation
|
@kszucs can you assist me with adapting ursabot for these changes? I think we can use pandas's Changes that would be good to have in
|
|
Just a small question: why are |
Sure.
I'm afraid this is not going to work, because we can't embed any CSS into the comment, this is why we generate the ursabot responses as diffs.
|
|
ursabot uses |
|
I’m sort of -1 on using anything but pandas for data munging and data presentation in our tooling. It’s not a very large dependency and has everything we need. FWIW, the current Ursabot output doesn't even sort the results, which is really needed to easily make sense of what got faster or slower at a glance. |
|
Using pandas is not a problem, but the results cannot be improved much other than sorting the table. |
|
I improved the output to show the |
|
+1. The bot changes can't be done here so going to go ahead and merge this so I can use it more easily without having to switch branches (to use this branch) before running benchmarks |
|
I’m going to update the bot tomorrow. |
This uses pandas to generate a sorted text table when using
archery benchmark diff. Example:#7506 (comment)
There's some other incidental changes
archery benchmark diff. I don't think there's value in reimplementing the stuff that pandas can do in a few lines of code (read JSON, create a sorted table and print it nicely for us).--repetitions=10on the command linearchery benchmarkwas building the unit tests unnecessarily. This also occluded a bug ARROW-9209, which is fixed here