-
Notifications
You must be signed in to change notification settings - Fork 645
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
Remove some code duplication in bindings #416
Conversation
39e97e0
to
00b434f
Compare
Thanks! Proposed few cosmetic changes |
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.
Oops. Forgot to send the review
python_bindings/bindings.cpp
Outdated
@@ -79,6 +79,45 @@ inline void assert_true(bool expr, const std::string & msg) { | |||
} | |||
|
|||
|
|||
inline void get_input_array_shapes(const py::buffer_info& buffer, size_t* rows, size_t* features) { | |||
if (buffer.ndim != 2 && buffer.ndim != 1) throw std::runtime_error("data must be a 1d/2d array"); |
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.
if (buffer.ndim != 2 && buffer.ndim != 1) throw std::runtime_error("data must be a 1d/2d array"); | |
if (buffer.ndim != 2 && buffer.ndim != 1) throw std::runtime_error("Input vector data wrong shape. Data must be a 1D or 2D array"); |
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 also added logging of actual number of dimensions in the error message
41a77e4
to
9921073
Compare
9921073
to
97e7009
Compare
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.
Looks good to me!
Introduced new functions
get_input_array_shapes
andget_input_ids_and_check_shapes
to remove code duplicationSmall refactoring: added spaces