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 Model.is_dynamic() method to Node.js API #23565

Closed
almilosz opened this issue Mar 20, 2024 · 8 comments · Fixed by #23646
Closed

[Good First Issue]: Expose Model.is_dynamic() method to Node.js API #23565

almilosz opened this issue Mar 20, 2024 · 8 comments · Fixed by #23646
Assignees
@qxprakash
Labels
category: JS API OpenVino JS API Bindings good first issue Good for newcomers gsoc-prerequisite-task Prerequisite task related to Google Summer of Code projects 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 Model.is_dynamic() method.
C++ API and Python API documentation of this method

What needs to be done?

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

Note: Remember about parameter validation

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

No response
@almilosz almilosz added good first issue Good for newcomers category: JS API OpenVino JS API Bindings no_stale Do not mark as stale gsoc-prerequisite-task Prerequisite task related to Google Summer of Code projects labels Mar 20, 2024
@almilosz almilosz self-assigned this Mar 20, 2024
@github-project-automation github-project-automation bot moved this to Contributors Needed in Good first issues Mar 20, 2024
@qxprakash
Copy link
Contributor

hello @almilosz I am interested in contributing to the JS API , I am taking this one as well if you don't mind ?

@qxprakash
Copy link
Contributor

.take

@github-actions GitHub Actions
Copy link
Contributor

Thanks for being interested in this issue. It looks like this ticket is already assigned to a contributor. Please communicate with the assigned contributor to confirm the status of the issue.

@almilosz
Copy link
Contributor Author

Hi Prakash,
I'm glad that you are so enthusiastic to contribute to OpenVINO Node.js API!
I reviewed PR for this GFI. Please take a look ;)

@qxprakash
Copy link
Contributor

Hi Prakash, I'm glad that you are so enthusiastic to contribute to OpenVINO Node.js API! I reviewed PR for this GFI. Please take a look ;)

Thanks for the feedback @almilosz , I have made the requested changes.

@qxprakash
Copy link
Contributor

qxprakash commented Mar 20, 2024 •
edited
Loading

@almilosz I had a question , the tests for Model Interface should be present in openvino/src/bindings/js/node/tests/basic.test.js right ? , I see tests for CompiledModel , Output class , Input class , but there are no tests for Model interface present ? , am I looking at the wrong location here ?

@p-wysocki p-wysocki assigned qxprakash and unassigned almilosz Mar 20, 2024
@p-wysocki p-wysocki moved this from Contributors Needed to Assigned in Good first issues Mar 20, 2024
@almilosz
Copy link
Contributor Author

You should create a new file called model.test.js and put there tests for Model.is_dynamic() (Node,js isDynamic())

@qxprakash
Copy link
Contributor

ok @almilosz , got it.

This was referenced Mar 25, 2024
Closed
Merged
@almilosz almilosz moved this from Assigned to In Review in Good first issues Mar 25, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 28, 2024
@qxprakash
[OV JS] Expose Model.isDynamic method to Node.js api (#23646)
877da91 
This fixes #23565 

Exposed Model.isDynamic() method to Node.js api

- Added parameter validation
- Added tests
@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 29, 2024
bbielawx pushed a commit to bbielawx/openvino that referenced this issue Apr 12, 2024
@qxprakash @bbielawx
[OV JS] Expose Model.isDynamic method to Node.js api (openvinotoolkit…
0d1e8be 
…#23646)

This fixes openvinotoolkit#23565 

Exposed Model.isDynamic() method to Node.js api

- Added parameter validation
- Added tests
alvoron pushed a commit to alvoron/openvino that referenced this issue Apr 29, 2024
@qxprakash @alvoron
[OV JS] Expose Model.isDynamic method to Node.js api (openvinotoolkit…
67e50c2 
…#23646)

This fixes openvinotoolkit#23565 

Exposed Model.isDynamic() method to Node.js api

- Added parameter validation
- Added tests
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 gsoc-prerequisite-task Prerequisite task related to Google Summer of Code projects no_stale Do not mark as stale
Projects
Good first issues
Archived in project
Milestone
2024.1
3 participants
@mlukasze @qxprakash @almilosz