Skip to content

Conversation

@almilosz
Copy link
Contributor

@almilosz almilosz commented Nov 26, 2025

Details:

  • Align methods in core.hpp to accept std::filesystem::path
  • [WIP] Add tests for fs::path and unicode cases

Tickets:

@github-actions github-actions bot added category: inference OpenVINO Runtime library - Inference category: CPP API OpenVINO CPP API bindings labels Nov 26, 2025
*/
void register_plugin(const std::string& plugin, const std::string& device_name, const ov::AnyMap& config = {});

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to duplicate comment it can be shared with previous version

Copy link
Contributor Author

@almilosz almilosz Nov 27, 2025

Choose a reason for hiding this comment

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

I added brief descriptions. Methods without comments didn't appear in our documentation e.g. :

template <class Path, std::enable_if_t<std::is_same_v<Path, std::filesystem::path>>* = nullptr>
auto read_model(const Path& model_path, const Path& bin_path = {}, const ov::AnyMap& properties = {}) const {

Copy link
Contributor

@praasz praasz Nov 27, 2025

Choose a reason for hiding this comment

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

There is like:

@{
@}
To group same functions with same description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it's broken in our documentation. It is shown as Unnamed Group https://docs.openvino.ai/2025/api/c_cpp_api/classov_1_1_core.html#_CPPv4NK2ov4Core10read_modelERKNSt6stringERKNSt6stringERKN2ov6AnyMapE and the signature of auto read_model(const Path& model_path, is not visible

Copy link
Contributor

Choose a reason for hiding this comment

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

Showing unamed group is OK

}

TEST_F(CoreBaseTest, LoadPluginXMLWithFsPath) {
std::string xml_file_name = "test_plugin.xml";
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not create it as path at start and modify it?
Is better to change this test to cover some unicode case see tests


std::string mock_plugin_name{"MOCK_HARDWARE_FS"};
std::filesystem::path plugin_path =
ov::util::make_plugin_library_name(ov::test::utils::getExecutableDirectory(),
Copy link
Contributor

Choose a reason for hiding this comment

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

IS path version called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Contributor

Choose a reason for hiding this comment

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

How as inputs are strings?

@praasz praasz self-assigned this Nov 27, 2025
@almilosz almilosz marked this pull request as ready for review November 27, 2025 22:26
@almilosz almilosz requested a review from a team as a code owner November 27, 2025 22:26
@almilosz almilosz requested a review from a team as a code owner November 28, 2025 19:53
@github-actions github-actions bot added the category: Python API OpenVINO Python bindings label Nov 28, 2025

cls.def("register_plugins",
&ov::Core::register_plugins,
static_cast<void (ov::Core::*)(const std::string&)>(&ov::Core::register_plugins),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should also consider how path is represented in Python API. In many places it's Union[str, bytes, pathlib.Path]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: CPP API OpenVINO CPP API bindings category: inference OpenVINO Runtime library - Inference category: Python API OpenVINO Python bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants