adding support for passing separate keyword arguments #189
adding support for passing separate keyword arguments #189arnavk23 wants to merge 19 commits intoJuliaSmoothOptimizers:mainfrom
Conversation
Co-authored-by: Tangi Migot <tangi.migot@gmail.com>
…o dirty for local tests
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #143 by adding support for passing separate keyword arguments to different levels of plotting function calls in performance profiles. The changes enable users to customize both individual performance profile plots and the final combined plot independently.
- Added
bp_kwargsparameter to pass kwargs toBenchmarkProfiles.performance_profilecalls - Added
plot_kwargsparameter to pass kwargs specifically to the finalPlots.plotcall - Added git status check to skip PkgBenchmark tests when the working tree has uncommitted changes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| test/pkgbmark.jl | Adds git status check to skip benchmarking when repository is dirty, preventing PkgBenchmark errors |
| src/profiles.jl | Introduces bp_kwargs and plot_kwargs parameters to allow fine-grained control over plotting keyword arguments at different function call levels |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Refactor tests to use CaptureBackend for profiling and plotting.
|
@tmigot Whenever you are free, please take a look. Thanks! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #189 +/- ##
==========================================
+ Coverage 76.71% 86.19% +9.48%
==========================================
Files 12 10 -2
Lines 292 297 +5
==========================================
+ Hits 224 256 +32
+ Misses 68 41 -27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
arnavk23
changed the title
kwargs
- Remove unused args... from performance_profile - Remove redundant bp_kwargs from performance_profile (both bp_kwargs and kwargs were splatted into same call) - Remove redundant plot_kwargs from profile_solvers (both plot_kwargs and kwargs were splatted into same call) - Update documentation to reflect simpler API - Update tests accordingly
dpo
left a comment
dpo
left a comment
There was a problem hiding this comment.
Thank you @arnavk23 and sorry for the long wait. This looks great. I suggest to pass bp_kwargs... last on calls to BenchmarkProfiles.performance_profile() so that the arguments in bp_kwargs can be used to override hard-coded arguments (such as palette = clrs, etc.). Let me know if that looks ok.
Co-authored-by: Dominique <dominique.orban@gmail.com>
dpo
dismissed
tmigot’s stale review
Can't find requested change and the pr looks fine now.
|
@arnavk23 Could you please rebase this branch? Thanks. |
|
@dpo Rebased the pr. |
|
@arnavk23 Github says there are still conflicts... I think the issue is that you have merge commits. Try to avoid those. Use |
|
closing in favor of #200 |
Fixes #143