Skip to content
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

pybind: replaced input type to bytearray - MOD-4597 #304

Merged
merged 18 commits into from
Dec 21, 2022

Conversation

meiravgri
Copy link
Collaborator

@meiravgri meiravgri commented Dec 13, 2022

Describe the changes in the pull request

The index function in bindings.cpp convert the py::object into a pointer directly instead of into an array

Which issues this PR fixes

  1. Previously the bindings functions explicitly declared an array of the required data type, to be later casted o a void*

Main objects this PR modified
1.bindings.cpp
2. test_hnswlib.py
3. test_bruteforce.py
4. bm_datasets.py

Mark if applicable

  • This PR introduces API changes
  • This PR introduces serialization changes

added test for fp32, fp64 and L2, cosine
@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Base: 96.58% // Head: 96.58% // No change to project coverage 👍

Coverage data is based on head (5a059d5) compared to base (a9c9cb9).
Patch has no changes to coverable lines.

❗ Current head 5a059d5 differs from pull request most recent head 7de6ce5. Consider uploading reports for the commit 7de6ce5 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #304   +/-   ##
=======================================
  Coverage   96.58%   96.58%           
=======================================
  Files          61       61           
  Lines        3482     3482           
=======================================
  Hits         3363     3363           
  Misses        119      119           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@meiravgri meiravgri changed the title pybind: replaced input type to bytearray pybind: replaced input type to bytearray - MOD-4597 Dec 13, 2022
tests/flow/Mybytearray.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@GuyAv46 GuyAv46 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice and clean!
I noticed you added an empty file (__init__.py) and few more comments

tests/flow/common.py Outdated Show resolved Hide resolved
tests/flow/test_bruteforce.py Outdated Show resolved Hide resolved
tests/flow/test_bruteforce.py Outdated Show resolved Hide resolved
tests/flow/test_bruteforce.py Outdated Show resolved Hide resolved
tests/flow/test_bruteforce.py Outdated Show resolved Hide resolved
tests/flow/test_bruteforce.py Show resolved Hide resolved
tests/flow/test_bruteforce.py Outdated Show resolved Hide resolved
@@ -69,24 +69,33 @@ class PyBatchIterator {
void reset() { VecSimBatchIterator_Reset(batchIterator.get()); }
virtual ~PyBatchIterator() {}
};
#include <pybind11/embed.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move include up the file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgot to remove it

@meiravgri meiravgri merged commit 0e8d38a into main Dec 21, 2022
@meiravgri meiravgri deleted the pybind_get_input_as_bytearray branch December 21, 2022 11:24
alonre24 pushed a commit that referenced this pull request Apr 4, 2023
* replaced input type to bytearray

* added a module to convert numpy to bytearay

* moved mybytearray to python bindings lib
alonre24 pushed a commit that referenced this pull request Apr 4, 2023
* replaced input type to bytearray

* added a module to convert numpy to bytearay

* moved mybytearray to python bindings lib
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants