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] Remove acceptable_types parameter from js_to_cpp() template function #24459

Merged
merged 15 commits into from May 20, 2024

Conversation

almilosz
Copy link
Contributor

Details:

  • Remove const std::vector<napi_types>& acceptable_types param from template TargetType js_to_cpp(const Napi::Value&, const std::vector<napi_types>& acceptable_types);
  • Remove template specialization int32_t js_to_cpp<int32_t>(const Napi::CallbackInfo& info, const size_t idx, const std::vector<napi_types>& acceptable_types);

Tickets:

  • 140428

@github-actions github-actions bot added the category: JS API OpenVino JS API Bindings label May 10, 2024
@almilosz almilosz marked this pull request as ready for review May 15, 2024 09:09
@almilosz almilosz requested a review from a team as a code owner May 15, 2024 09:09
src/bindings/js/node/src/helper.cpp Outdated Show resolved Hide resolved
Comment on lines +80 to 82
} else {
OPENVINO_THROW("Passed argument must be of type Array or TypedArray.");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If else is last instruction in the function body we can white it out of condition

Suggested change
} else {
OPENVINO_THROW("Passed argument must be of type Array or TypedArray.");
}
}
OPENVINO_THROW("Passed argument must be of type Array or TypedArray.");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to keep it for the sake of readability

@@ -184,7 +145,7 @@ ov::preprocess::ResizeAlgorithm js_to_cpp<ov::preprocess::ResizeAlgorithm>(
} else if (algorithm == "RESIZE_LINEAR") {
return ov::preprocess::ResizeAlgorithm::RESIZE_LINEAR;
} else {
OPENVINO_THROW(std::string("Not supported resizeAlgorithm."));
OPENVINO_THROW("Not supported resizeAlgorithm.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Possible to release from brackets

if (!acceptableType(elem, acceptable_types)) {
OPENVINO_THROW(std::string("Cannot convert Napi::Value to std::map<std::string, ov::Any>"));
}
OPENVINO_ASSERT(elem.IsObject(), "Cannot convert Napi::Value to std::map<std::string, ov::Any>");
Copy link
Contributor

Choose a reason for hiding this comment

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

Element at index idx should be an Object type

@mlukasze mlukasze added this to the 2024.2 milestone May 16, 2024
@almilosz almilosz enabled auto-merge May 17, 2024 13:32
@almilosz almilosz added this pull request to the merge queue May 20, 2024
Merged via the queue into openvinotoolkit:master with commit 0eb8a42 May 20, 2024
114 checks passed
@almilosz almilosz deleted the almilosz/js-to-cpp branch May 20, 2024 16:39
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 Code Freeze
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants