-
Notifications
You must be signed in to change notification settings - Fork 169
[NVIDIA] Prepare setup.py for PyPi #1001
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
Changes from all commits
f7becb0
2995a1a
42c4f44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,3 +8,4 @@ dist/ | |
|
|
||
| report_api.xml | ||
| report_op.xml | ||
| wheel/packages/openvino_nvidia/lib/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,22 +18,44 @@ def _get_lib_file_extension() -> str: | |
| return "dylib" | ||
|
|
||
|
|
||
| def _register_nvidia_plugin(): | ||
| def _register_nvidia_plugin(force=False): | ||
| import openvino | ||
| openvino_package_dir = os.path.dirname(os.path.abspath(openvino.__file__)) | ||
| openvino_package_libs_dir = os.path.join(openvino_package_dir, "libs") | ||
| openvino_nvidia_gpu_package_dir = os.path.dirname(os.path.abspath(__file__)) | ||
| openvino_nvidia_gpu_library = os.path.join(openvino_nvidia_gpu_package_dir, f"../libopenvino_nvidia_gpu_plugin.{_get_lib_file_extension()}") | ||
| openvino_nvidia_gpu_library = os.path.join(openvino_nvidia_gpu_package_dir, f"./lib/libopenvino_nvidia_gpu_plugin.{_get_lib_file_extension()}") | ||
|
|
||
| xml_file = os.path.join(openvino_package_libs_dir, "plugins.xml") | ||
| xml_file_updated = False | ||
| tree = ET.parse(xml_file).getroot() | ||
| plugins = tree.find("plugins") | ||
| if all(plugin.get('name') != 'NVIDIA' for plugin in plugins.iter('plugin')): | ||
| plugins.append(Element('plugin', {'name': 'NVIDIA', 'location': openvino_nvidia_gpu_library})) | ||
|
|
||
| if force: | ||
| for plugin in plugins: | ||
| if plugin.tag == "plugin" and plugin.get("name") == "NVIDIA": | ||
| plugins.remove(plugin) | ||
| plugins.append(Element('plugin', {'name': 'NVIDIA', 'location': openvino_nvidia_gpu_library})) | ||
| xml_file_updated = True | ||
| else: | ||
| if all(plugin.get('name') != 'NVIDIA' for plugin in plugins.iter('plugin')): | ||
| plugins.append(Element('plugin', {'name': 'NVIDIA', 'location': openvino_nvidia_gpu_library})) | ||
| xml_file_updated = True | ||
|
|
||
| if xml_file_updated: | ||
| with open(xml_file, "w") as f: | ||
| f.write(ET.tostring(tree).decode('utf8')) | ||
|
|
||
|
|
||
| def __getattr__(name): | ||
| if name == "install": | ||
| _register_nvidia_plugin() | ||
| elif name == "force_install": | ||
| _register_nvidia_plugin(True) | ||
| else: | ||
| raise AttributeError(f"module {__name__} has no attribute {name}") | ||
|
|
||
|
|
||
| _register_nvidia_plugin() | ||
|
|
||
| __version__ = "2024.1.0" | ||
|
|
||
| __version__ = "2025.3.0" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps we should be setting this dynamically based on One way or another, if we'd like to set this dynamically, this is a topic for a different PR.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@p-wysocki Have not idea, I am not the official maintainer of NVIDIA plugin, I am the former developer of it and I know pretty good its architecture. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| openvino==2024.1.0 | ||
| openvino==2025.3.0 |
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.
Calling
bdist_wheelmanually has been deprecated for a while, going as far as an official Python blog saying that they mustn't be run anymore. The recommended replacement ispython -m buildwhich requiresbuilddependency. Could you please check if running it is compatible and works well? We shouldn't introduce functionality based on outdated, possibly broken command.Same goes for
setup.py installbelow.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.
The functionality could be done iteratively.
In my current python environment
python -m buildfailed due to missed dependencies (probably because I have custom build for python)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.
I'm not a fan of introducing a functionality which we know may be broken/is not only not recommended, but soft-forbidden by Python docs. These things tend to stick around as a "TODO" and later cause issues at the most inconvenient time.
I haven't maintained NVIDIA plugin before so IMO my opinion can be overridden if you wish to do so (cc @p-durandin). If we decide to merge it as-is at the very least let's create a ticket to update the build method.
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.
@p-wysocki thanks, lets merge with creating github issue