-
-
Notifications
You must be signed in to change notification settings - Fork 623
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
Added: Improved tests using pytest parametrize #2543
Conversation
…se.py using pytest parametrize
@nmcguire101 thanks for the PR, however I'm not sure if this change is an improvement. This test with asserts I think does not require any reparametrization. You can also see that your PR removes 14 lines and adds 19 lines. |
b6b511c
to
a03cacc
Compare
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.
Thanks for the update @nmcguire101
Few naming nits and good to go, I think
Co-authored-by: vfdev <[email protected]>
Co-authored-by: vfdev <[email protected]>
Co-authored-by: vfdev <[email protected]>
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.
Thanks @nmcguire101
LGTM
@nmcguire101 would you mind to apply similar ideas to other tests ? |
Yes, I will do that. Thank you for all your help |
Related to #2522 (edited by @vfdev-5 to avoid closing the issue when this PR is merged)
Description:
Improved test tests\ignite\contrib\metrics\regression\test__base.py using pytest parametrize
Check list: