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

[OV][JS] Expose the Tensor.isContinuous to Node.js API #27710

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nashez
Copy link
Contributor

@nashez nashez commented Nov 23, 2024

Details:

  • Add a TensorWrap::is_continuous function: Calls the underlying Tensor.isContinous function
  • Update the addon.ts file with the isContinuous method
  • Add unit tests for the isContinuous Api

Tickets:

Closes: #27701

@nashez nashez requested a review from a team as a code owner November 23, 2024 10:17
@github-actions github-actions bot added the category: JS API OpenVino JS API Bindings label Nov 23, 2024
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Nov 23, 2024
@nashez
Copy link
Contributor Author

nashez commented Nov 23, 2024

@almilosz Please review the changes.

@nashez nashez force-pushed the nashez/napi_tensor_continuous branch from bd45732 to 29840b8 Compare November 24, 2024 09:45
Copy link
Contributor

@vishniakov-nikolai vishniakov-nikolai left a comment

Choose a reason for hiding this comment

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

@nashez thank you for contribution!

src/bindings/js/node/lib/addon.ts Outdated Show resolved Hide resolved
@vishniakov-nikolai vishniakov-nikolai added this to the 2025.0 milestone Nov 25, 2024
@nashez
Copy link
Contributor Author

nashez commented Nov 25, 2024

Thanks for the approval @vishniakov-nikolai

Could you please trigger a CI run for this ?
There might be some uncaught clang format related issues on the PR because my machine doesn't support the clang-format-fixall command.

If there are any issues, I will resolve them along with your existing comment and push later today

@nashez nashez force-pushed the nashez/napi_tensor_continuous branch from 29840b8 to 4db89e0 Compare November 25, 2024 09:41
@almilosz
Copy link
Contributor

build_jenkins

@nashez nashez force-pushed the nashez/napi_tensor_continuous branch from d00695d to 0bd54fc Compare December 9, 2024 07:16
@nashez
Copy link
Contributor Author

nashez commented Dec 9, 2024

@almilosz @vishniakov-nikolai Made the requested changes. Please review.

@nashez nashez force-pushed the nashez/napi_tensor_continuous branch 4 times, most recently from 6450c34 to 5366f65 Compare December 12, 2024 18:12
@nashez
Copy link
Contributor Author

nashez commented Dec 12, 2024

@almilosz @vishniakov-nikolai I see the CI failing.
Is it some issue in the PR or some other issue with the master branch?

@vishniakov-nikolai
Copy link
Contributor

@almilosz @vishniakov-nikolai I see the CI failing. Is it some issue in the PR or some other issue with the master branch?

Hi @nashez! Sometimes it happens, don't worry. I'll take care about it and let you know if some changes in your brunch will be needed.

* Add a TensorWrap::is_continuous function:
  Calls the underlying Tensor.isContinous function
* Update the addon.ts file with the isContinuous method
* Add unit tests for the isContinuous Api

Closes: openvinotoolkit#27701

Signed-off-by: Nashez Zubair <nashezzubair@gmail.com>
@nashez nashez force-pushed the nashez/napi_tensor_continuous branch from 456e142 to 17975ad Compare December 19, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: JS API OpenVino JS API Bindings ExternalPR External contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Good First Issue]: Expose Tensor.isContinuous() functionality
4 participants