-
Notifications
You must be signed in to change notification settings - Fork 102
Automatically create nested BenchmarkGroup
on access
#309
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
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'm not a developer/maintainer of BenchmarkTools.jl and can't really judge this. All I can say is that you should also add some tests.
Thanks. Will definitely add tests; just wanted to have this PR “fail fast” if this feature is a nonstarter |
I'm not a developer/maintainer, so I can't comment on this |
@vchuravy would you be able to take a look at this? I can add tests if this looks okay as a feature. |
@ararslan would you be up for taking a look at this? |
@vchuravy just made me a maintainer (hurray 🤣) and it turns out I ran into this very problem last week, so I'm gonna say the feature is a welcome one. Please add tests and I'll review! |
Done! |
I need to fix CI cause #305 broke it for unsupported Julia versions, then we'll re-run the tests |
OK tests are failing outside of 1.5 and 1.7, that's not on me :) |
Oops, fixed! |
@MilesCranmer do you think this is a breaking change or a new feature? |
I think it’s a breaking change, so worth bumping the version number |
OK then I'll wait until we fix a few longstanding bugs |
@gdalle I noticed a new version has not been released yet and I was wondering if you might be able to do so? Would be great to use this feature! |
I have been hesitant cause it's a central package and breaking changes have been made, but a recent discussion on slack convinced me to move forward. Waiting on one PR then I'll do a bit of cleanup and tag 2.0 |
Fixes #308
This lets you perform, e.g.,
without needing to manually create each benchmark group. Normally you would have to do something like:
which seems tedious.
This is similar to how EasyConfig.jl works, in that
getindex
will automatically create each requiredBenchmarkGroup()
.The downside is that accessing the wrong key will create empty groups. Thus, there is the
clear_empty!
command to clean these up (I added docs too).