-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
MAINT Create private _pairwise_distances_reductions
submodule
#23724
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
MAINT Create private _pairwise_distances_reductions
submodule
#23724
Conversation
Those interfaces are meant to be used in the Python code, decoupling the actual implementation from the Python code. This allows changing all the private implementation while maintaining a contract for the Python callers. Each interface extending the base `PairwiseDistancesReduction` interface must implement the :meth:`compute` classmethod. Under the hood, such a function must only define the logic to dispatch at runtime to the correct dtype-specialized `PairwiseDistancesReduction` implementation based on the dtype of X and of Y. This refactoring will ease other dtype support such as float32 support.
… with some ASCII art. 🎨 Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Jérémie du Boisberranger <jeremiedbb@users.noreply.github.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Jérémie du Boisberranger <jeremiedbb@users.noreply.github.com> Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Also reword comments in tests
From: #14 (review) Co-authored-by: Jérémie du Boisberranger <jeremiedbb@users.noreply.github.com>
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 like the new organization. A lot more readable !
What's the purpose of having an empty test
directory inside the _pairwise_distances_reduction
sub-module ?
Co-authored-by: Jérémie du Boisberranger <jeremiedbb@users.noreply.github.com>
Thanks for the review. It definitely was time to.
Just a re-factoring left-over. |
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!
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
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.
Thank you for the PR! I like the refactor.
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
…eds] test_chunk_size_agnosticism test_n_threads_agnosticism test_strategies_consistency test_pairwise_distances_argkmin test_pairwise_distances_radius_neighbors test_sqeuclidean_row_norms Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
test_chunk_size_agnosticism test_n_threads_agnosticism test_strategies_consistency test_pairwise_distances_argkmin test_pairwise_distances_radius_neighbors test_sqeuclidean_row_norms
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
…it-learn#23724) Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org> Co-authored-by: Jérémie du Boisberranger <jeremiedbb@users.noreply.github.com> Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Co-authored-by: Jérémie du Boisberranger <34657725+jeremiedbb@users.noreply.github.com>
Reference Issues/PRs
Follow-up of #23515.
What does this implement/fix? Explain your changes.
The single source file is split in several files to create a private submodule:
tree
submodule)Any other comments?