Skip to content

Fix #30149: Fix Pyright errors in utils/decorators.py #30194

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

Fix #30149: Fix Pyright errors in utils/decorators.py #30194

wants to merge 12 commits into from

Conversation

sanleo-wq
Copy link

Description

This PR fixes issue #30149 by resolving the Pyright errors in the src/openvino/utils/decorators.py file.

Changes made

  • Corrected return type annotation of _get_name function from Node to Optional[str]
  • Added proper type annotations to the methods in the MultiMethod class
  • Added safety checks for issubclass calls to prevent type errors
  • Fixed type annotations in the signature access
  • Added null check for better error handling

Testing

  • Removed src/openvino/utils/decorators.py from the exclusions in pyproject.toml
  • Ran Pyright which now reports no errors:

@sanleo-wq sanleo-wq requested a review from a team as a code owner April 17, 2025 15:31
@github-actions github-actions bot added the category: Python API OpenVINO Python bindings label Apr 17, 2025
@sys-openvino-ci sys-openvino-ci added the ExternalPR External contributor label Apr 17, 2025
mlukasze
mlukasze previously requested changes Apr 18, 2025
View reviewed changes
src/bindings/python/src/openvino/utils/decorators.py Outdated

from openvino import Node, Output
from openvino.utils.types import NodeInput, as_node, as_nodes


def _get_name(**kwargs: Any) -> Node:
def _get_name(**kwargs: Any) -> Optional[str]: # Corretto il tipo di ritorno
Copy link
Contributor

Choose a reason for hiding this comment

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

if needed, comments should be in english, please :)

src/bindings/python/src/openvino/utils/decorators.py Outdated
Comment on lines 83 to 85
elif get_origin(type_arg) == list:
if issubclass(actual_type, list):
if isinstance(actual_type, type) and issubclass(actual_type, list):
return True
Copy link
Contributor

@mlukasze mlukasze Apr 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

it could be one-line condition, elif itself makes no sense here, imho, nesting for sure
@akuporos what's your opinion?

src/bindings/python/src/openvino/utils/decorators.py Outdated
return True
return False

def matches_optional(self, optional_type, actual_type) -> bool: # type: ignore
def matches_optional(self, optional_type: Any, actual_type: Any) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary change

@mlukasze mlukasze linked an issue Apr 18, 2025 that may be closed by this pull request
[Good First Issue]: Fix pyright in: src/openvino/utils/decorators.py #30149
Closed
@p-wysocki
Copy link
Contributor

p-wysocki commented Apr 18, 2025 •
edited
Loading

Thank you for the PR! Could you please also regenerate .pyi files as mentioned in the issue description? All you need to do is build OpenVINO with -DENABLE_WHEEL=ON, install that wheel python -m pip install build/wheels/openvino... and either:

  • make any commit, and the .pyi files will be automatically regenerated and added to your commit
  • run python src/bindings/python/scripts/generate_stub_files.py and git add them manually

@mlukasze mlukasze dismissed their stale review April 22, 2025 05:11

requested changes done, removing roadblock, enabling CI

akuporos
akuporos reviewed Apr 22, 2025
View reviewed changes
src/bindings/python/src/openvino/utils/decorators.py Outdated
@@ -115,14 +114,14 @@ def __call__(self, *args, **kwargs) -> Any: # type: ignore
break
elif len(arg_types) == 0 and len(kwarg_types) != 0:
for key, func in self.typemap.items():
func_signature = {arg_name: types.annotation for arg_name, types in signature(func).parameters.items()}
func_signature = {arg_name: param.annotation for arg_name, param in signature(func).parameters.items()}
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you rename variables here?

@p-wysocki
Copy link
Contributor

Hello @sanleo-wq, thank for your your PR! There are some unintended changes to .pyi files which turned out to be a bug on our side. Could you please regenerate these files and commit them?

There's no need to rebuild the project, you can just apply the changes to stub generation Python script from the linked PR and run python src/bindings/python/scripts/generate_pyapi_stubs.py.

Also, src/bindings/python/src/openvino/__init__.pyi has some peculiar changes - it removed everything from the main namespace and replaced it with an empty list. Could you please share what CMake you're using to build OpenVINO?

akuporos
akuporos reviewed Apr 24, 2025
View reviewed changes
src/bindings/python/src/openvino/utils/decorators.py Outdated
@@ -122,7 +121,7 @@ def __call__(self, *args, **kwargs) -> Any: # type: ignore
break
elif len(arg_types) != 0 and len(kwarg_types) != 0:
for key, func in self.typemap.items():
func_signature = {arg_name: types.annotation for arg_name, types in signature(func).parameters.items()}
func_signature = {arg_name: param.annotation for arg_name, param in signature(func).parameters.items()}
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 still param instead of types

@github-actions github-actions bot added the category: TF FE OpenVINO TensorFlow FrontEnd label Apr 24, 2025
@p-wysocki
Copy link
Contributor

Thank you for the investigation! The tasks for fixing pyright seemed straightforward, but when reviewing the PRs we decided that it's a much more difficult issue than we expected and, combined with the task priority, we decided to move fixes for pyright checks back as an internal OpenVINO team effort. It's no longer an open Good First Issue - we apologize for closing the task you were working on, your contribution helped us investigate that issue.

@p-wysocki p-wysocki closed this Apr 25, 2025
@sanleo-wq sanleo-wq deleted the fix-30149 branch April 28, 2025 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akuporos akuporos akuporos left review comments

@mlukasze mlukasze mlukasze left review comments

Assignees
No one assigned
Labels
category: Python API OpenVINO Python bindings category: TF FE OpenVINO TensorFlow FrontEnd ExternalPR External contributor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Good First Issue]: Fix pyright in: src/openvino/utils/decorators.py
5 participants
@sanleo-wq @p-wysocki @akuporos @mlukasze @sys-openvino-ci