Skip to content

[Python API] quick fix of packaging#8870

Merged
azhogov merged 10 commits intoopenvinotoolkit:masterfrom
akuporos:akup/quick-fix-packaging
Nov 30, 2021
Merged

[Python API] quick fix of packaging#8870
azhogov merged 10 commits intoopenvinotoolkit:masterfrom
akuporos:akup/quick-fix-packaging

Conversation

@akuporos
Copy link
Copy Markdown
Contributor

@akuporos akuporos commented Nov 27, 2021

Details:

  • Move content from openvino -> runtime
  • As a quick solution it works like from openvino.runtime import Core

Tickets:

  • 71835

image

@akuporos akuporos requested a review from a team November 27, 2021 15:11
@ilya-lavrenov ilya-lavrenov self-assigned this Nov 27, 2021
@ilya-lavrenov ilya-lavrenov added this to the 2022.1 milestone Nov 27, 2021
@openvino-pushbot openvino-pushbot added the category: inference OpenVINO Runtime library - Inference label Nov 27, 2021
@akuporos akuporos added bug Something isn't working category: Python API OpenVINO Python bindings labels Nov 27, 2021
@akuporos akuporos force-pushed the akup/quick-fix-packaging branch 2 times, most recently from 8e7a38f to 0ac4989 Compare November 27, 2021 15:38
@akuporos akuporos force-pushed the akup/quick-fix-packaging branch from 0ac4989 to 3f507c7 Compare November 27, 2021 16:07
@akuporos akuporos force-pushed the akup/quick-fix-packaging branch from e15982b to 7480e05 Compare November 27, 2021 17:28
@akuporos akuporos requested a review from a team November 27, 2021 22:40
@akuporos akuporos force-pushed the akup/quick-fix-packaging branch from ff724cf to ad84670 Compare November 27, 2021 22:53
from openvino.runtime.impl import PartialShape
from openvino.runtime.impl import Layout

from openvino.runtime.ie_api import Core
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could you try to create "root" aliases?

 from openvino.runtime.ie_api import Core
+openvino.Core = Core

This should repair from openvino import Core


To have openvino symbol please try:

  • openvino = sys.modules['openvino']
  • or just import openvino (in general it returns cached value from sys.modules)

BTW, Do we have import openvino.Core somewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alalek ok, thank you, I'll try it.

BTW, Do we have import openvino.Core somewhere?

As I know and as I saw in files, no, we don't have import openvino.Core

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alalek ok, I've tried to add openvino.Core = Core inside openvino/runtime/__init__.py and also openvino = sys.modules['openvino']

But the same problem occurred.
Attach console
image

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks like the root __init__.py doesn't really import sub-packages during initialization (please add some debug "print" into runtime/__init__.py to check that).


Can we add dummy subpackages for Core, Tensor, etc?

openvino/Core/__init__.py:

import openvino

from openvino.runtime.ie_api import Core
openvino.Core = Core
del sys.modules['openvino.Core']

Need to check that these cases work with workaround:

  • import openvino.Core
  • from openvino import Core

Copy link
Copy Markdown

@alalek alalek Nov 28, 2021

Choose a reason for hiding this comment

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

We could initialize sub-packages using the walk_packages approach (available in Python 3.0+ at least)

Root openvino/__init__.py:

 __path__ = __import__('pkgutil').extend_path(__path__, __name__)

+def _subpackage_import_error(name):
+    print("WARN: Error importing sub-package '%s'" % name)
+    pass
+
+for module_info in __import__('pkgutil').walk_packages(__path__, __name__ + '.', onerror=_subpackage_import_error):
+    print(module_info.name)
+    pass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@alalek
I may try but looks like other subpackages like accuracy_checker should also add these lines.
Also one small doubt came from here: https://packaging.python.org/guides/packaging-namespace-packages/#pkgutil-style-namespace-packages

The __init__.py file for the namespace package needs to contain only the following:

__path__ = __import__('pkgutil').extend_path(__path__, __name__)

As I know, the code after this line is inaccessible.

BTW, this PR is like quick fix to unblock all, I'll continue to make from openvino import Core work here: #8846

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

the code after this line is inaccessible.

My local print() experiment shows that code before and after this line runs properly.
There is no anything special - just regular Python code.

However it it critical that all openvino/__init__.py files must have the same content. Including AC.


if we need to initialize "runtime" only (and we perform all necessary initialization in runtime/__init__.py), then this would be enough (without walk_packages):

 __path__ = __import__('pkgutil').extend_path(__path__, __name__)
+__import__(__name__ + '.runtime')  # force initialization

AC possible workaround for __init__:

  • use openvino -> openvino_tools package
  • AC's openvino_tools/__init__.py - no special requirements except .extend_path()
  • OV's openvino/__init__.py: add second "extend_path" line:
     __path__ = __import__('pkgutil').extend_path(__path__, __name__)
    +__path__ = __import__('pkgutil').extend_path(__path__, 'openvino_tools')
    +__import__(__name__ + '.runtime')

Verify (not sure about AC actual name):

  • import openvino.accuracy_checker
  • from openvino import accuracy_checker
  • import openvino without installed openvino_tools

from openvino import Core, Tensor
from openvino.impl import Function
from openvino.runtime import Core, Tensor
from openvino.runtime.impl import Function
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

from openvino.runtime.impl import Function

src/bindings/python/src/openvino/runtime/__init__.py file has this:

from openvino.runtime.impl import Function

and we don't have __all__ there.

So, this should work:

-from openvino.runtime.impl import Function
+from openvino.runtime import Function

or with aliases above

-from openvino.runtime.impl import Function
+from openvino import Function

with the similar result.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

from openvino.runtime import Core, Tensor, Function

To make this line work and import all 3 elements we also need to add in runtime/__init__.py (besides of added openvino.Core = Core):

openvino.Tensor = Tensor
openvino.Function = Function

Copy link
Copy Markdown
Contributor Author

@akuporos akuporos Nov 28, 2021

Choose a reason for hiding this comment

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

@alalek I just wanted to make sure that this way would help with Core import. If it helped me, I'd add the rest classes.
BTW, still I need to import firstly openvino.runtime, only after that from openvino import Core works. So looks like when the user call import openvino, only openvino/__init__.py is initialized while openvino/runtime/__init__.py will be initialized after direct call of import openvino.runtime.
Look at the comment above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

but it works as it imports like from openvino.runtime

Copy link
Copy Markdown

@alalek alalek Nov 28, 2021

Choose a reason for hiding this comment

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

I agree that we need try to check one part first.

I expected this diff (from initially commented patch):

-from openvino.runtime import Core, Tensor
+from openvino import Core
+from openvino.runtime import Tensor
 from openvino.runtime.impl import Function

BTW, Could you please add link to builder which contains logs for these lines?


only openvino/__init__.py is initialized

Looks like you are right (please check separate thread above).

@akuporos akuporos force-pushed the akup/quick-fix-packaging branch from cfa1659 to a330706 Compare November 28, 2021 11:55
@akuporos akuporos force-pushed the akup/quick-fix-packaging branch from a330706 to 5096c1a Compare November 28, 2021 13:18
@azhogov azhogov merged commit f6df0a9 into openvinotoolkit:master Nov 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 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.

5 participants