-
Notifications
You must be signed in to change notification settings - Fork 7.2k
Cleanup prototype transforms tests #6984
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
Conversation
| *, | ||
| agg_method=None, | ||
| allowed_percentage_diff=None, | ||
| mae=False, |
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 ported both agg_method and allowed_percentage diff from the stable tests. In the prototype tests we only ever used agg_method="mean". This just makes it more expressive by adding an mae=False flag instead.
|
|
||
|
|
||
| def make_info_args_kwargs_parametrization(infos, *, args_kwargs_fn, condition=None): | ||
| if condition is None: |
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.
There is no need to have a condition that filters infos here, if we can simply filter them before passing them here.
| expected, | ||
| **info.get_closeness_kwargs(test_id, dtype=input.dtype, device=input.device), | ||
| msg=parametrized_error_message(*other_args, *kwargs), | ||
| msg=parametrized_error_message(*other_args, **kwargs), |
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 botched this when I introduced the extra information. Without the ** we would only include the names of the parameters rather than the name-value-pair.
datumbox
left a comment
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!
Summary: * minor cleanup of the prototype transforms tests * refactor ImagePair * pretty format enum Reviewed By: YosuaMichael Differential Revision: D41648543 fbshipit-source-id: 7d7aacda750d994ad6bfa22f42dec92b18e16ccc
This PR cleans up some minor thing in the prototype transforms test suite. I'm going to highlight them with inline comments.
cc @vfdev-5 @datumbox @bjuncek