Add 'bencher' console-script entry point for the render/compare CLI#964
Merged
Conversation
The render/compare CLI was only reachable via 'python -m bencher.render', which is non-obvious and easy to mistype. Register a 'bencher' console script (project.scripts -> bencher.render:main) so it can be invoked as 'bencher <result.pkl> <out> [--json PATH]' and 'bencher compare ...'. Purely additive: 'python -m bencher.render' is unchanged. Usage/--help text is now invocation-aware via a _prog() helper so the displayed command matches how the tool was run.
Contributor
Reviewer's GuideAdds a new Sequence diagram for bencher CLI invocation pathssequenceDiagram
actor User
participant Shell
participant bencher_script as bencher_console_script
participant python_module as bencher.render
participant main as main
participant prog as _prog
User->>Shell: bencher ...
Shell->>bencher_script: execute entry point
bencher_script->>python_module: import bencher.render
bencher_script->>main: main(argv)
main->>prog: _prog()
prog-->>main: bencher
main-->>User: usage/help with bencher
alt python -m bencher.render
User->>Shell: python -m bencher.render ...
Shell->>python_module: python -m bencher.render
python_module->>main: main(argv)
main->>prog: _prog()
prog-->>main: python -m bencher.render
main-->>User: usage/help with python -m bencher.render
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
_prog()helper assumes the console script name is exactlybencher, which may not hold on some platforms or packaging setups (e.g.,bencher.exe,bencher-3.12); consider a more robust detection (e.g.,exe.startswith("bencher")or checkingsys.argv[0]for a suffix) so usage/help stays accurate in those cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_prog()` helper assumes the console script name is exactly `bencher`, which may not hold on some platforms or packaging setups (e.g., `bencher.exe`, `bencher-3.12`); consider a more robust detection (e.g., `exe.startswith("bencher")` or checking `sys.argv[0]` for a suffix) so usage/help stays accurate in those cases.
## Individual Comments
### Comment 1
<location path="bencher/render.py" line_range="142" />
<code_context>
+ to the explicit ``python -m bencher.render`` form otherwise, so the usage
+ line always names a command the reader can actually run.
+ """
+ exe = Path(sys.argv[0]).name
+ return "bencher" if exe == "bencher" else "python -m bencher.render"
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle Windows console-script suffixes when deriving the program name.
On Windows, installed console scripts are usually invoked via `bencher.exe` or `bencher-script.py`, so `Path(sys.argv[0]).name` will rarely equal `"bencher"`. That means `_prog()` will still report `"python -m bencher.render"`, even when users actually run `bencher`, which makes the usage text misleading for Windows users.
To avoid this, make the comparison extension-agnostic, for example by stripping common suffixes before comparing, or simply using `Path(sys.argv[0]).stem` instead of `.name` when checking for `"bencher"`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Performance Report for
|
| Metric | Value |
|---|---|
| Total tests | 1681 |
| Total time | 123.71s |
| Mean | 0.0736s |
| Median | 0.0020s |
Top 10 slowest tests
| Test | Time (s) |
|---|---|
test.test_bench_examples.TestBenchExamples::test_example_meta |
17.120 |
test.test_over_time_save_perf::test_save_faster_without_aggregated_tab |
5.072 |
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_drift.py] |
4.519 |
test.test_generated_examples::test_generated_example[cartesian_animation/example_cartesian_animation.py] |
2.965 |
test.test_split_render_examples::test_split_render_subprocess_media |
2.953 |
test.test_hash_persistent.TestCrossProcessDeterminism::test_hash_stable_across_two_processes[ResultBool] |
2.815 |
test.test_over_time_repeats.TestMaxSliderPoints::test_default_subsampling_caps_at_max |
2.810 |
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_noise.py] |
2.787 |
test.test_generated_examples::test_generated_example[result_types/result_image/example_result_image_to_video.py] |
2.777 |
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_step.py] |
2.491 |
Updated by Performance Tracking workflow
Match a 'bencher' prefix on argv[0] basename rather than an exact 'bencher' so platform/packaging variants of the console script still resolve the usage text correctly: bencher.exe on Windows, bencher-3.12 for a versioned install. The module form's render.py basename never matches. Addresses PR review.
Performance Report for
|
| Metric | Value |
|---|---|
| Total tests | 1681 |
| Total time | 124.41s |
| Mean | 0.0740s |
| Median | 0.0020s |
Top 10 slowest tests
| Test | Time (s) |
|---|---|
test.test_bench_examples.TestBenchExamples::test_example_meta |
17.703 |
test.test_over_time_save_perf::test_save_faster_without_aggregated_tab |
4.969 |
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_drift.py] |
4.547 |
test.test_split_render_examples::test_split_render_subprocess_media |
2.989 |
test.test_generated_examples::test_generated_example[cartesian_animation/example_cartesian_animation.py] |
2.948 |
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_noise.py] |
2.785 |
test.test_generated_examples::test_generated_example[result_types/result_image/example_result_image_to_video.py] |
2.782 |
test.test_hash_persistent.TestCrossProcessDeterminism::test_hash_stable_across_two_processes[ResultBool] |
2.724 |
test.test_generated_examples::test_generated_example[regression/example_regression_tuning_step.py] |
2.493 |
test.test_over_time_repeats.TestMaxSliderPoints::test_default_subsampling_caps_at_max |
2.409 |
Updated by Performance Tracking workflow
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
What
Register a
bencherconsole-script entry point so the render/compare CLI is a real command instead of onlypython -m bencher.render.After install:
Why
python -m bencher.renderis non-obvious and easy to mistype — the module namerenderdoesn't hint that it also does JSON export and result comparison. A first-time user (or an agent told to "use the bencher CLI") has to go discover the module path. A named command matches the mental model and makesbencher --helpself-documenting.Non-breaking
This is purely additive:
python -m bencher.render …is unchanged and keeps working — samemain()._prog()helper: it readsbencherunder the console script andpython -m bencher.renderfrom a source checkout, so the displayed command always matches how the tool was run.Verification
test/test_render.py— addedtest_prog_name_is_invocation_aware; full module green (22 passed). Existing CLI tests driverender.main([...])directly and are unaffected.bencherscript resolves and dispatches:bencher --help→usage: bencher …bencher compare --help→usage: bencher compare …Usage: bencher <result_path> <output_dir> [--json PATH], exit 2ruff check/ruff format/prek runall clean.Version bumped
1.107.0 → 1.108.0(minor — new feature) with a CHANGELOG entry, per the auto-publish-on-version-bump convention.Summary by Sourcery
Introduce a named bencher CLI entry point while keeping the existing python -m bencher.render invocation working unchanged.
New Features:
Enhancements:
Build:
Documentation:
Tests: