-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[Py OV] Move ie_api.py and exceptions.py files to openvino module #28007
[Py OV] Move ie_api.py and exceptions.py files to openvino module #28007
Conversation
Signed-off-by: Alicja Miloszewska <[email protected]>
Signed-off-by: Alicja Miloszewska <[email protected]>
Signed-off-by: Alicja Miloszewska <[email protected]>
Signed-off-by: Alicja Miloszewska <[email protected]>
Signed-off-by: Alicja Miloszewska <[email protected]>
…into almilosz/move-py-files Signed-off-by: Alicja Miloszewska <[email protected]>
Signed-off-by: Alicja Miloszewska <[email protected]>
Signed-off-by: Alicja Miloszewska <[email protected]>
Do we need to change |
Currently there are no changes to |
from openvino.runtime import CompiledModel | ||
from openvino.runtime import InferRequest | ||
from openvino.runtime import AsyncInferQueue | ||
from openvino._ov_api import Model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the correct (official and offered to user) way to import Model
, Core
, and others here?
May be it should be just from openvino import Model
and from openvino import Core
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is
_ov_api
contains python wrappers above pybind11 bindings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me, _ov_api
looks like internal api that nobody should use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a check that __init__
files of openvino, ovc and benchmark app (prev also mo and openvino-dev) should be the same. It's still openvino/__init__.py
. All three __init__
files during wheel building are installed in the same place openvino/__init__
. If they aren't identical, they will overload each other.
@@ -43,12 +43,13 @@ | |||
from openvino.runtime import Tensor | |||
from openvino.runtime import OVAny | |||
|
|||
from openvino.runtime import compile_model | |||
# Helper functions for openvino module | |||
from openvino.runtime.utils.data_helpers import tensor_from_file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we still have openvino.runtime
? May be it is better to have openvino.utils
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will be done in a separate PR
from openvino.runtime import compile_model | ||
# Helper functions for openvino module | ||
from openvino.runtime.utils.data_helpers import tensor_from_file | ||
from openvino._ov_api import compile_model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User will never import in such a way. AFAIK, we propose from openvino import compile_model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
answered above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear. Let us use from openvino import compile_model
. Now it should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_ov_api.py
is just a place for Python wrappers for C++ bindings (sometimes they're necessary, not in all cases). Since all __init__.py
files are merged into one in wheel, they have to be aligned - that's why that change is also in tools/ovc/openvino/__init__.py
.
Because it's in __init__.py
, this import is indeed "internal" as you mentioned - the user will still be able to run from openvino import compile_model
, because that __init__.py
pulls compile_model
from _ie_api.py
and publishes it in the main openvino
module.
The act of publishing the function to the main module itself is internal, it's not public API and should not concern the user, who keeps the same functionality of from openvino import compile_model
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please clarify what is a concept of having _ov_api
? Can't we just import directly from openvino
?
To sum up From the user perspective it's a change a like this:
It's a bad practice to have the implementation of classes and functions in |
@rkazants @akuporos @p-wysocki |
I am lost. So do you mean I need to use Best regards, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need clarification of further Python API design and not clear why _ov_api
is needed and targeted for.
from openvino.runtime import compile_model | ||
# Helper functions for openvino module | ||
from openvino.runtime.utils.data_helpers import tensor_from_file | ||
from openvino._ov_api import compile_model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_ov_api.py
is just a place for Python wrappers for C++ bindings (sometimes they're necessary, not in all cases). Since all __init__.py
files are merged into one in wheel, they have to be aligned - that's why that change is also in tools/ovc/openvino/__init__.py
.
Because it's in __init__.py
, this import is indeed "internal" as you mentioned - the user will still be able to run from openvino import compile_model
, because that __init__.py
pulls compile_model
from _ie_api.py
and publishes it in the main openvino
module.
The act of publishing the function to the main module itself is internal, it's not public API and should not concern the user, who keeps the same functionality of from openvino import compile_model
.
…envinotoolkit#28007) ### Details: - move `runtime/ie_api.py` and rename it to `_ov_api.py` as ie_api is a legacy name. - add `runtime/ie_apy/__init__.py` as proxy - update `openvino/__init__` to import from `_ov_api.py`. - move `runtime/exceptions.py` and add proxy in `runtime` - update `test_ovdict` to use `OVDict` defined in `openvino.runtime.utils.data_helpers` - update `tensor_from_file` import in `openvino/__init__.py`. This function is located in `openvino.runtime.utils.data_helpers` Part of `openvino.runtime` deprecation ### Tickets: - [CVS-129459](https://jira.devtools.intel.com/browse/CVS-129459) --------- Signed-off-by: Alicja Miloszewska <[email protected]>
### Details: Add deprecation warning for `openvino.runtime` that will be shown **ONCE** when sb will access runtime functionality for the first time. Examples:   `openvino.runtime` funtionality was added to openvino namespace in these PRs: - #27785 - #27902 - #28007 - #28062 - #28085 Internal calls in `openvino` module also triggered warning, so they were updated: - #28166 - #28356 ### Tickets: - [CVS-129451](https://jira.devtools.intel.com/browse/CVS-129451) --------- Signed-off-by: Alicja Miloszewska <[email protected]> Co-authored-by: Anastasia Kuporosova <[email protected]>
…7694) ### Details: Add deprecation warning for `openvino.runtime` that will be shown **ONCE** when sb will access runtime functionality for the first time. Examples:   `openvino.runtime` funtionality was added to openvino namespace in these PRs: - openvinotoolkit#27785 - openvinotoolkit#27902 - openvinotoolkit#28007 - openvinotoolkit#28062 - openvinotoolkit#28085 Internal calls in `openvino` module also triggered warning, so they were updated: - openvinotoolkit#28166 - openvinotoolkit#28356 ### Tickets: - [CVS-129451](https://jira.devtools.intel.com/browse/CVS-129451) --------- Signed-off-by: Alicja Miloszewska <[email protected]> Co-authored-by: Anastasia Kuporosova <[email protected]>
Details:
move
runtime/ie_api.py
and rename it to_ov_api.py
as ie_api is a legacy name.add
runtime/ie_apy/__init__.py
as proxyupdate
openvino/__init__
to import from_ov_api.py
.move
runtime/exceptions.py
and add proxy inruntime
update
test_ovdict
to useOVDict
defined inopenvino.runtime.utils.data_helpers
update
tensor_from_file
import inopenvino/__init__.py
. This function is located inopenvino.runtime.utils.data_helpers
Part of
openvino.runtime
deprecationTickets: