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.

Sign up for GitHub

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

Jump to bottom

[Good First Issue]: Expose Tensor.get_size() method to Node.js API #23440

Closed
almilosz opened this issue Mar 13, 2024 · 10 comments · Fixed by #23498
Closed

[Good First Issue]: Expose Tensor.get_size() method to Node.js API #23440

almilosz opened this issue Mar 13, 2024 · 10 comments · Fixed by #23498
Assignees
@qxprakash
Labels
category: JS API OpenVino JS API Bindings good first issue Good for newcomers no_stale Do not mark as stale
Milestone
2024.1

Comments

@almilosz
Copy link
Contributor

Context

Node.js API is the newest binding of OpenVINO.
The task is to expose Tensor.get_size() method.
C++ API and Python API documentation of this method

What needs to be done?

  • add get_size() method on C++ side (src/bindings/js/node/src/tensor.cpp)
  • update TypeScript definition (src/bindings/js/node/lib/addon.ts)
  • create unit test for added functionality using Node.js Test Runner

Example Pull Requests

Here is the link to a PR with a very similar task that covers the steps listed above:

Resources

Contact points

@almilosz @vishniakov-nikolai

Ticket

134826
@almilosz almilosz added good first issue Good for newcomers no_stale Do not mark as stale labels Mar 13, 2024
@github-project-automation github-project-automation bot moved this to Contributors Needed in Good first issues Mar 13, 2024
@vishniakov-nikolai vishniakov-nikolai added the category: JS API OpenVino JS API Bindings label Mar 13, 2024
@hiirrxnn
Copy link

Hi , can I work on this ? Thank you.

@qxprakash
Copy link
Contributor

Hi @almilosz I would like to work on this issue thanks.

@qxprakash
Copy link
Contributor

.take

@github-actions GitHub Actions
Copy link
Contributor

Thank you for looking into this issue! Please let us know if you have any questions or require any help.

@mlukasze mlukasze moved this from Contributors Needed to Assigned in Good first issues Mar 14, 2024
@vishniakov-nikolai
Copy link
Contributor

Hi , can I work on this ? Thank you.

Hi @hiirrxnn, thank you for your interest! Unfortunately this issue has taken, but this JS API issue is still waiting to be assigned: https://github.com/orgs/openvinotoolkit/projects/3/views/1?pane=issue&itemId=56296668, feel free to take it!

@qxprakash
Copy link
Contributor

qxprakash commented Mar 16, 2024 •
edited
Loading

hello @vishniakov-nikolai , Ideally I should also validate the parameters of Tensor.get_size() right ? , if user passes any arguments it should throw an error somewhat similar to the python api ?
TypeError: get_size(): incompatible function arguments. The following argument types are supported: 1. (self: openvino._pyopenvino.Tensor) -> int

@qxprakash qxprakash mentioned this issue Mar 16, 2024
Merged
@qxprakash
Copy link
Contributor

qxprakash commented Mar 17, 2024 •
edited
Loading

@vishniakov-nikolai , do you have other unassigned JS-api issues which are not being worked upon ? , I would like to contribute to Node.js API , Thanks.

@mlukasze
Copy link
Contributor

Hey @qxprakash !

Nikolai is on short vacation but on behalf of him I can guarantee that more tasks related to JS bindings is coming :)
Stay tune and check our JS GFI Project board from time to time :)

@qxprakash
Copy link
Contributor

qxprakash commented Mar 18, 2024 •
edited
Loading

hello @mlukasze , Thanks , I am interested in contributing to the Node.js API , I will keep an eye on the board.

@mlukasze
Copy link
Contributor

Alicja will take a look soon, please stay tuned! :)

@mlukasze mlukasze moved this from Assigned to In Review in Good first issues Mar 19, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 28, 2024
@qxprakash @almilosz @vishniakov-nikolai
Added Tensor.get_size() method to Node.js API (#23498)
496a5de 
### This Fixes #23440
 
### Details:

Extended Tensor API , with tensor.getSize() method

-  Implemented tensor.getSize() in the js api
-  added parameter validation
-  updated Tensor interface with the getSize() method in addon.ts
-  added Tests

---------

Co-authored-by: Alicja Miloszewska <[email protected]>
Co-authored-by: Vishniakov Nikolai <[email protected]>
@github-project-automation github-project-automation bot moved this from In Review to Closed in Good first issues Mar 28, 2024
@mlukasze mlukasze added this to the 2024.1 milestone Mar 28, 2024
bbielawx pushed a commit to bbielawx/openvino that referenced this issue Apr 12, 2024
@qxprakash @almilosz
@vishniakov-nikolai @bbielawx
Added Tensor.get_size() method to Node.js API (openvinotoolkit#23498)
a975279 
### This Fixes openvinotoolkit#23440
 
### Details:

Extended Tensor API , with tensor.getSize() method

-  Implemented tensor.getSize() in the js api
-  added parameter validation
-  updated Tensor interface with the getSize() method in addon.ts
-  added Tests

---------

Co-authored-by: Alicja Miloszewska <[email protected]>
Co-authored-by: Vishniakov Nikolai <[email protected]>
alvoron pushed a commit to alvoron/openvino that referenced this issue Apr 29, 2024
@qxprakash @almilosz
@vishniakov-nikolai @alvoron
Added Tensor.get_size() method to Node.js API (openvinotoolkit#23498)
9cc43b9 
### This Fixes openvinotoolkit#23440
 
### Details:

Extended Tensor API , with tensor.getSize() method

-  Implemented tensor.getSize() in the js api
-  added parameter validation
-  updated Tensor interface with the getSize() method in addon.ts
-  added Tests

---------

Co-authored-by: Alicja Miloszewska <[email protected]>
Co-authored-by: Vishniakov Nikolai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@qxprakash qxprakash

Labels
category: JS API OpenVino JS API Bindings good first issue Good for newcomers no_stale Do not mark as stale
Projects
Good first issues
Archived in project
Milestone
2024.1
Development

Successfully merging a pull request may close this issue.

Added Tensor.get_size() method to Node.js API qxprakash/openvino
5 participants
@mlukasze @vishniakov-nikolai @qxprakash @almilosz @hiirrxnn