Skip to content
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

Add a error message when creating an empty Constant #19674

Merged
merged 6 commits into from
Sep 13, 2023
Merged

Add a error message when creating an empty Constant #19674

merged 6 commits into from
Sep 13, 2023

Conversation

siddhant-0707
Copy link
Contributor

@siddhant-0707 siddhant-0707 commented Sep 7, 2023 •
edited by ilya-lavrenov
Loading

Details:

  • Added an error message when creating an empty constant to the constant wrapper

Tickets:

@siddhant-0707
add error msg empty constant
1f4b69a 
added an error msg when creating an empty constant to the constant wrapper
@siddhant-0707 siddhant-0707 requested a review from a team as a code owner September 7, 2023 12:15
@github-actions github-actions bot added the category: Python API OpenVINO Python bindings label Sep 7, 2023
p-wysocki
p-wysocki approved these changes Sep 7, 2023
View reviewed changes
Copy link
Contributor

@p-wysocki p-wysocki left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! LGTM. @siddhant-0707 have you tested it locally to see if the ValueError is raised?

There are currently some issues with CI, please do not worry that some jobs are failing.

@p-wysocki p-wysocki mentioned this pull request Sep 7, 2023
Closed
@siddhant-0707
Copy link
Contributor Author

@p-wysocki Yes, ValueError raised correctly. I tested it on Python 3.10.12

akuporos
akuporos reviewed Sep 7, 2023
View reviewed changes
Copy link
Contributor

@akuporos akuporos left a comment

Choose a reason for hiding this comment

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

Can you please also extend the test here, that error raised?

@ilya-lavrenov ilya-lavrenov added the ExternalPR External contributor label Sep 8, 2023
@siddhant-0707 @akuporos
made reviewed changes
00e1b62 
Co-authored-by: Anastasia Kuporosova <[email protected]>
@jiwaszki jiwaszki enabled auto-merge (squash) September 12, 2023 11:55
@akuporos
Copy link
Contributor

build_jenkins

@siddhant-0707
Copy link
Contributor Author

Hey, was wondering if I need to do something else also to merge this PR?

@akuporos
Copy link
Contributor

Hey, was wondering if I need to do something else also to merge this PR?

Actually no, just wait for the last job will pass

@akuporos
Copy link
Contributor

build_jenkins

@jiwaszki jiwaszki merged commit 4ca3d51 into openvinotoolkit:master Sep 13, 2023
@siddhant-0707 siddhant-0707 deleted the empty-constant-error branch October 25, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akuporos akuporos akuporos approved these changes

@p-wysocki p-wysocki p-wysocki approved these changes

@jiwaszki jiwaszki jiwaszki approved these changes

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

Successfully merging this pull request may close these issues.

Add a clear error message when creating an empty Constant
5 participants
@siddhant-0707 @akuporos @p-wysocki @jiwaszki @ilya-lavrenov