[Python API] Fix: Create constant for strings and fix segfault by KarSri7694 · Pull Request #33574 · openvinotoolkit/openvino

KarSri7694 · 2026-01-13T10:27:20Z

This PR fixes #23611

The numpy based construtor now handles numpy-array with string as elements.
converts None to empty string ''

Related tests have also been added to test_constant.py.

This reverts commit bfb1a1c.

- Now it will make "" instead of "None"

…crash but gives a warning when shared_memory = true and data is string

KarSri7694 · 2026-01-13T10:29:48Z

Hello, I would request maintainers to review this code and please tell me if any more changes are required.
Also, is None handling done right?
currently it just converts None to ""

The code currently handles shared_memory = True, by giving a warning using std::cerr, is this correct way?

p-wysocki

Overall LGTM, thank you for your contribution! I added a few comments before approval.

p-wysocki · 2026-01-13T11:24:14Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp

+                             std::cerr << "Warning: Creating a String Constant with shared memory is not supported. "
+                                          "Data will be copied."
+                                       << std::endl;


<iostream> is a big header which we don't really need included in our bindings. Instead of std::cerr please use OPENVINO_WARN macro. Example usage: https://github.com/openvinotoolkit/openvino/blob/master/src/bindings/python/src/pyopenvino/graph/node_factory.cpp#L92

p-wysocki · 2026-01-13T11:24:48Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp

+                         }
+                         // convert NumPy array to flattened list of strings
+                         std::string* tensor_data = tensor.data<std::string>();
+                         //  std::vector<std::string> strings;


Suggested change

// std::vector<std::string> strings;

KarSri7694 · 2026-01-13T11:31:36Z

Hey @p-wysocki , I will resolve the issue by today

Copilot

Pull request overview

This PR fixes a segmentation fault when creating constants from NumPy arrays containing strings. It adds support for string-based NumPy arrays in the Python API constructor and converts None values to empty strings.

Changes:

Added string array handling logic in the C++ constant constructor to detect and process string/object dtype arrays
Implemented conversion of None values to empty strings and type conversion for mixed-type object arrays
Added comprehensive test coverage for string constants including edge cases (empty arrays, None values, Unicode, mixed types)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/bindings/python/src/pyopenvino/graph/ops/constant.cpp	Implements string array detection and conversion logic in the constant constructor
src/bindings/python/tests/test_graph/test_constant.py	Adds 8 test cases covering various string constant scenarios

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-13T12:31:32Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp

+                         auto flat_array = array.reshape({-1});
+
+                         // copy data
+                         for (int i = 0; i < flat_array.size(); ++i) {


Use size_t or py::ssize_t instead of int for the loop counter. The flat_array.size() returns a size type, and using int could cause issues with large arrays or sign comparison warnings.

Suggested change

for (int i = 0; i < flat_array.size(); ++i) {

for (py::ssize_t i = 0; i < flat_array.size(); ++i) {

Copilot · 2026-01-13T12:31:33Z

src/bindings/python/tests/test_graph/test_constant.py

+    # Note: Warning goes to stderr, check if it appears
+    captured = capfd.readouterr()
+    # The warning should be printed to stderr
+    assert "Warning" in captured.err or len(captured.err) == 0  # May not always capture in test


This assertion doesn't reliably verify the warning behavior. The condition or len(captured.err) == 0 makes the test pass even when no warning is produced, defeating the purpose of testing the warning. Consider using pytest's warns context manager or making the assertion more strict.

Copilot · 2026-01-13T12:31:33Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp

+                         }
+                         // convert NumPy array to flattened list of strings
+                         std::string* tensor_data = tensor.data<std::string>();
+                         //  std::vector<std::string> strings;


Remove commented-out code. This appears to be leftover from development and should be cleaned up.

Suggested change

// std::vector<std::string> strings;

- OPENVINO_WARN outputs to stderr and is not visible when calling from python, so this method is now used. It is still kept in code and can be used while debugging when ENABLE_OPENVINO_DEBUG flag is set to ON

…loop

KarSri7694 · 2026-01-13T13:43:00Z

Hey @p-wysocki, I have made the necessary changes, please review.

I have added both OPENVINO_WARN and PyErr_WarnEx.
OPENVINO_WARN can be useful in debugging scenarios and PyErr_WarnEx is used to show RuntimeWarning in python.

I have also updated the test_constant.py to detect updated RuntimeWarning using pytest.warns()

p-wysocki · 2026-01-13T16:20:01Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp

+                if (shared_memory) {
+                    OPENVINO_WARN(
+                        "Creating a String Constant with shared memory is not supported. Data will be copied.");
+                    PyErr_WarnEx(PyExc_RuntimeWarning,
+                                 "Creating a String Constant with shared memory is not supported. Data will be copied.",
+                                 1);
+                }


Suggested change

if (shared_memory) {

OPENVINO_WARN(

"Creating a String Constant with shared memory is not supported. Data will be copied.");

PyErr_WarnEx(PyExc_RuntimeWarning,

"Creating a String Constant with shared memory is not supported. Data will be copied.",

1);

}

if (shared_memory) {

OPENVINO_WARN(

"Creating a String Constant with shared memory is not supported. Data will be copied.");

}

-The test currently does not assert the statements caught by capfd, this is because the logs are getting supressed in windows build and i cannot find a way to fix it

KarSri7694 · 2026-01-14T13:08:26Z

Hello,
I have removed the PyErr_WarnEx code as asked, I have also omitted the assert code in test_constant as I am unable to produce logs even after setting OPENVINO_LOG_LEVEL=2 in the environment in my windows build. I think windows might be supressing the logs. Any help is appreciated

p-wysocki · 2026-01-15T08:46:04Z

@KarSri7694 I see you attempted it with captured.err, but that's not the recommended way. Could you please try using with pytest.warns? There are numerous examples in the Python tests.

p-wysocki · 2026-01-15T11:13:55Z

Linters failed:

tests/test_graph/test_constant.py:419:121: E501 line too long (154 > 120 characters)
        [4.75, 4.5, -5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, -0.1, -0.2, -0.3, -0.4, -0.5, -0.6, -0.7, -0.8, -0.9, -1, 448, 512],
                                                                                                                        ^
tests/test_graph/test_constant.py:538:121: E501 line too long (153 > 120 characters)
        [4.75, 4.5, 5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0, 448, 512, np.nan],
                                                                                                                        ^
tests/test_graph/test_constant.py:552:121: E501 line too long (160 > 120 characters)
    target = [4.0, 4.0, 4.0, 0.0, 0.125, 0.25, 0.25, 0.5, 0.5, 0.5, 0.5, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 2.0, 2.0, 2.0, 2.0, 2.0, 2.0, 512, 512, np.nan]
                                                                                                                        ^
tests/test_graph/test_constant.py:661:121: E501 line too long (154 > 120 characters)
        [4.75, 4.5, -5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, -0.1, -0.2, -0.3, -0.4, -0.5, -0.6, -0.7, -0.8, -0.9, -1, 448, 512],
                                                                                                                        ^
tests/test_graph/test_constant.py:722:121: E501 line too long (153 > 120 characters)
        [4.75, 4.5, 5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0, 448, 512, np.nan],
                                                                                                                        ^
tests/test_graph/test_constant.py:737:121: E501 line too long (160 > 120 characters)
    target = [4.0, 4.0, 4.0, 0.0, 0.125, 0.25, 0.25, 0.5, 0.5, 0.5, 0.5, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 2.0, 2.0, 2.0, 2.0, 2.0, 2.0, 512, 512, np.nan]
                                                                                                                        ^
tests/test_graph/test_constant.py:1029:5: F841 local variable 'captured' is assigned to but never used
    captured = capfd.readouterr()
    ^

You can run linters by going to the Python directory and running python -m flake8 tests/ --config=setup.cfg.

KarSri7694 · 2026-01-15T13:48:31Z

@KarSri7694 I see you attempted it with captured.err, but that's not the recommended way. Could you please try using with pytest.warns? There are numerous examples in the Python tests.

I looked into using pytest.warns, but there is a compatibility issue:

pytest.warns only captures Python Warning objects (generated by warnings.warn or the C-API PyErr_WarnEx).
OPENVINO_WARN logs directly to the C++ standard output stream. It does not raise a Python exception or warning object, so pytest.warns ignores it completely.

To use pytest.warns as requested, I would need to revert the implementation to use PyErr_WarnEx. To keep OPENVINO_WARN, I must use capfd to verify the log output.

Please let me know which implementation you prefer.

KarSri7694 · 2026-01-15T14:01:20Z

Linters failed:

tests/test_graph/test_constant.py:419:121: E501 line too long (154 > 120 characters)
        [4.75, 4.5, -5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, -0.1, -0.2, -0.3, -0.4, -0.5, -0.6, -0.7, -0.8, -0.9, -1, 448, 512],
                                                                                                                        ^
tests/test_graph/test_constant.py:538:121: E501 line too long (153 > 120 characters)
        [4.75, 4.5, 5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0, 448, 512, np.nan],
                                                                                                                        ^
tests/test_graph/test_constant.py:552:121: E501 line too long (160 > 120 characters)
    target = [4.0, 4.0, 4.0, 0.0, 0.125, 0.25, 0.25, 0.5, 0.5, 0.5, 0.5, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 2.0, 2.0, 2.0, 2.0, 2.0, 2.0, 512, 512, np.nan]
                                                                                                                        ^
tests/test_graph/test_constant.py:661:121: E501 line too long (154 > 120 characters)
        [4.75, 4.5, -5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, -0.1, -0.2, -0.3, -0.4, -0.5, -0.6, -0.7, -0.8, -0.9, -1, 448, 512],
                                                                                                                        ^
tests/test_graph/test_constant.py:722:121: E501 line too long (153 > 120 characters)
        [4.75, 4.5, 5.25, 0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1, -0.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9, 2.0, 448, 512, np.nan],
                                                                                                                        ^
tests/test_graph/test_constant.py:737:121: E501 line too long (160 > 120 characters)
    target = [4.0, 4.0, 4.0, 0.0, 0.125, 0.25, 0.25, 0.5, 0.5, 0.5, 0.5, 1.0, 1.0, 1.0, 0.0, 1.0, 1.0, 1.0, 1.0, 2.0, 2.0, 2.0, 2.0, 2.0, 2.0, 512, 512, np.nan]
                                                                                                                        ^
tests/test_graph/test_constant.py:1029:5: F841 local variable 'captured' is assigned to but never used
    captured = capfd.readouterr()
    ^

You can run linters by going to the Python directory and running python -m flake8 tests/ --config=setup.cfg.

Will fix it

…ning test

… and improve test case

KarSri7694 · 2026-01-19T09:24:51Z

Hey @p-wysocki, I have implemented the pytest.warns function in test_constant.py file
Linting errors are also fixed.
Please review.

p-wysocki

LGTM, please take a look at my last nit. Thank you for your contribution!

p-wysocki · 2026-01-28T09:11:22Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp


 #include "openvino/core/shape.hpp"
 #include "openvino/runtime/tensor.hpp"
+#include "openvino/util/log.hpp"


Which utility is this required for? PyErr_WarnEx is used, but it's a pybind util. If it's not necessary, please remove it.

Previously I was using OPENVINO_WARN, then switched to PyErr_WarnEx, but forgot to remove the header, Will remove its no longer required.

KarSri7694 · 2026-01-28T10:40:25Z

I have made the required changes and removed log header.

KarSri7694 · 2026-02-10T17:52:55Z

Hey @p-wysocki ,
Just a gentle follow-up on this PR.
Please let me know if any changes or updates are required from my side.
Thanks!

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Copilot · 2026-02-25T08:36:03Z

src/bindings/python/src/pyopenvino/graph/ops/constant.cpp

+                     return std::make_shared<ov::op::v0::Constant>(
+                         Common::object_from_data<ov::op::v0::Constant>(array, shared_memory));


The return statement creates a Constant from another Constant object returned by object_from_data. Since object_from_data already returns an ov::op::v0::Constant, wrapping it in another std::make_shared<ov::op::v0::Constant>() is redundant and may cause issues. Return the result directly instead.

Suggested change

return std::make_shared<ov::op::v0::Constant>(

Common::object_from_data<ov::op::v0::Constant>(array, shared_memory));

return Common::object_from_data<ov::op::v0::Constant>(array, shared_memory);

KarSri7694 added 13 commits January 11, 2026 23:18

Initial support for numpy array containg strings

978c4b1

Prevent creation of String Constant with shared memory = true

2724080

Add handling for empty string

5e5c9d9

Handle None values in string array conversion for OpenVINO Constant

a90a9a1

fix code formatting

bfb1a1c

Revert "fix code formatting"

0ab39d0

This reverts commit bfb1a1c.

fixes code formatting and fix one missing bracket in 'if' block

60ec248

refactor for case when data=[None]

32ccb73

- Now it will make "" instead of "None"

The code now throws error with useful message

ed9fddc

fix for bad memory allocation during tests. the program now does not …

3bb875b

…crash but gives a warning when shared_memory = true and data is string

Add tests for numpy based string constant creation

bfb5002

fix code formatting

eea2be1

revert change in tensor-based constructor

5356296

KarSri7694 requested a review from a team as a code owner January 13, 2026 10:27

github-actions bot added the category: Python API OpenVINO Python bindings label Jan 13, 2026

sys-openvino-ci added the ExternalPR External contributor label Jan 13, 2026

p-wysocki reviewed Jan 13, 2026

View reviewed changes

almilosz requested a review from Copilot January 13, 2026 12:30

Copilot AI reviewed Jan 13, 2026

View reviewed changes

KarSri7694 added 5 commits January 13, 2026 18:08

remove unused code

8ac201d

Use openvino_warn macro instead of std::cerr to log warnings

c850790

Adds PyErr_WarnEx to show runtime warning to user

b1d8898

- OPENVINO_WARN outputs to stderr and is not visible when calling from python, so this method is now used. It is still kept in code and can be used while debugging when ENABLE_OPENVINO_DEBUG flag is set to ON

use ssize_t instead of int to handle potentailly large arrays in for …

aef640a

…loop

fix code formatting

bafcff1

remove unused iostream include

b129cb3

p-wysocki reviewed Jan 13, 2026

View reviewed changes

remove PyErr_WarnEx

6e8e95f

Refactor shared_memory test

71423d7

-The test currently does not assert the statements caught by capfd, this is because the logs are getting supressed in windows build and i cannot find a way to fix it

KarSri7694 added 3 commits January 15, 2026 19:44

enable correct assertion output for string constant shared memory war…

0c72a4c

…ning test

fix: update string constant shared memory warning to use PyErr_WarnEx…

8ecc58c

… and improve test case

fix code formatting

f7b873d

p-wysocki approved these changes Jan 28, 2026

View reviewed changes

p-wysocki mentioned this pull request Jan 28, 2026

Python API: prevent string tensor constants to avoid crash #33659

Closed

remove unused log header

ac48a77

KarSri7694 requested a review from p-wysocki February 21, 2026 05:34

mlukasze requested a review from Copilot February 25, 2026 08:31

Copilot AI reviewed Feb 25, 2026

View reviewed changes

Copilot started reviewing on behalf of mlukasze February 25, 2026 09:18 View session

	for (int i = 0; i < flat_array.size(); ++i) {
	for (py::ssize_t i = 0; i < flat_array.size(); ++i) {

		return std::make_shared<ov::op::v0::Constant>(
		Common::object_from_data<ov::op::v0::Constant>(array, shared_memory));

	return std::make_shared<ov::op::v0::Constant>(
	Common::object_from_data<ov::op::v0::Constant>(array, shared_memory));
	return Common::object_from_data<ov::op::v0::Constant>(array, shared_memory);

Conversation

KarSri7694 commented Jan 13, 2026

Uh oh!

KarSri7694 commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p-wysocki left a comment

Choose a reason for hiding this comment

Uh oh!

p-wysocki Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

p-wysocki Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

KarSri7694 commented Jan 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

KarSri7694 commented Jan 13, 2026

Uh oh!

p-wysocki Jan 13, 2026

Choose a reason for hiding this comment

Uh oh!

KarSri7694 commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p-wysocki commented Jan 15, 2026

Uh oh!

p-wysocki commented Jan 15, 2026

Uh oh!

KarSri7694 commented Jan 15, 2026

Uh oh!

KarSri7694 commented Jan 15, 2026

Uh oh!

KarSri7694 commented Jan 19, 2026

Uh oh!

p-wysocki left a comment

Choose a reason for hiding this comment

Uh oh!

p-wysocki Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

KarSri7694 Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

KarSri7694 commented Jan 28, 2026

Uh oh!

KarSri7694 commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KarSri7694 commented Jan 13, 2026 •

edited

Loading

KarSri7694 commented Jan 14, 2026 •

edited

Loading