Skip to content

Conversation

@Huanli-Gong
Copy link
Contributor

This fixes #23567
What has been done:

  • add get_output_size() 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

@Huanli-Gong Huanli-Gong requested a review from a team as a code owner April 2, 2024 22:29
@github-actions github-actions bot added the category: JS API OpenVino JS API Bindings label Apr 2, 2024
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.

Minor comments

@vishniakov-nikolai vishniakov-nikolai added this to the 2024.2 milestone Apr 4, 2024
Co-authored-by: Vishniakov Nikolai <[email protected]>
Copy link
Contributor

@almilosz almilosz left a comment

Choose a reason for hiding this comment

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

The CIs are failing because there is a possible loss of data when you convert size_t to double.
Consider representing output_size as different Napi::Value

auto-merge was automatically disabled April 17, 2024 18:12

Head branch was pushed to by a user without write access

Copy link
Contributor

@almilosz almilosz left a comment

Choose a reason for hiding this comment

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

After fixing merge conflicts it's ready to merge. Be careful not to remove get/set FriendlyName() methods

@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Apr 23, 2024
@Huanli-Gong Huanli-Gong requested a review from almilosz April 23, 2024 15:29
@vishniakov-nikolai
Copy link
Contributor

build_jenkins

@almilosz almilosz added this pull request to the merge queue May 13, 2024
Merged via the queue into openvinotoolkit:master with commit 69dea7c May 13, 2024
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 Model.get_output_size() method to Node.js API

5 participants