-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
DOC Add airspeed benchmarks page link to the contributing docs #22075
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
DOC Add airspeed benchmarks page link to the contributing docs #22075
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.
@ohadmich Thanks for this PR.
doc/developers/contributing.rst
Outdated
@@ -934,8 +934,9 @@ Monitoring performance | |||
When proposing changes to the existing code base, it's important to make sure | |||
that they don't introduce performance regressions. Scikit-learn uses | |||
`asv benchmarks <https://github.com/airspeed-velocity/asv>`_ to monitor the | |||
performance of a selection of common estimators and functions. The benchmark | |||
suite can be found in the `scikit-learn/asv_benchmarks` directory. | |||
performance of a selection of common estimators and functions, you can view |
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.
performance of a selection of common estimators and functions, you can view | |
performance of a selection of common estimators and functions. You can view |
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.
Thanks for the suggestion! Now added :)
doc/developers/contributing.rst
Outdated
suite can be found in the `scikit-learn/asv_benchmarks` directory. | ||
performance of a selection of common estimators and functions, you can view | ||
these benchmarks on the `scikit-learn benchmark page <https://scikit-learn.org/scikit-learn-benchmarks>`_. | ||
The benchmark suite can be found in the `scikit-learn/asv_benchmarks` directory. |
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.
The benchmark suite can be found in the `scikit-learn/asv_benchmarks` directory. | |
The corresponding benchmark suite can be found in the `scikit-learn/asv_benchmarks` directory. |
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.
This makes things a lot clearer, thanks! Added as well.
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.
LGTM
Reference Issues/PRs
What does this implement/fix? Explain your changes.
Fixes #21898
Added a link to the airspeed benchmarks page in
doc/developers/contributing.rst
under the intro paragraph of the Monitoring Performance section as suggested by @ogriselAny other comments?
I think it should be pretty clear now, but please let me know if you would like me to change anything about the phrasing. Thanks!