-
Notifications
You must be signed in to change notification settings - Fork 615
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
Add error message checking into existing python tests #3324
Conversation
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
with assert_raises(RuntimeError): | ||
_test_err_args(classes=[0,1,2,3], weights=np.float32([1,2,3])) | ||
with assert_raises(RuntimeError): | ||
_test_err_args(classes=np.int32([0,1,2,3]), weights=[3,2,1]) |
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.
Renamed kwargs
weights -> class_weights
ignore_classes -> ignore_class
to meet api names, I believe that was intended check here.
!build |
CI MESSAGE: [2939211]: BUILD STARTED |
CI MESSAGE: [2939211]: BUILD PASSED |
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.
Just some nitpicks.
expected_error_msgs = ( | ||
common_msg.format("a callable that does not accept arguments"), | ||
"External source callback must be a callable with 0 or 1 argument", | ||
common_msg.format("an iterable"), | ||
common_msg.format("a generator function")) |
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.
Any reason for using tuple instead of list of the expected error msgs?
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.
Not really, subconscious preference I guess.
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.
done
with assert_raises(RuntimeError): | ||
pipe.build() | ||
pipe.run() | ||
expected_errors = ( |
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.
Same here about tuple vs list.
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.
done
yield raises(TypeError, | ||
"`output_dtypes` should be provided as single tf.DType value or a tuple of tf.DType values")( | ||
dali_pipe_deprecated), { "shapes": 2, }, 2, tf.uint8, dali_types.UINT8, 1, 2 |
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.
If any fixes appear, pleas add the missing newline
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.
done
{ "shapes": 2, "output_shapes": 2, "dtypes": tf.uint8 }, 2, tf.uint8, dali_types.UINT8, 1, 2 | ||
yield dali_pipe_deprecated_raises, \ | ||
error_msg = "Usage of `{}` is deprecated in favor of `output_{}`*only `output_{}` should be provided." | ||
yield raises(ValueError, error_msg.format(*(("shapes",) * 3)))(dali_pipe_deprecated), \ |
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.
maybe
with assert_raises(ValueError, glob=error_msg.format(...)):
...
would be less of a parentheses rollecoaster?
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.
added named variables for error message strings
Signed-off-by: Kamil Tokarski <ktokarski@nvidia.com>
!build |
CI MESSAGE: [2969924]: BUILD STARTED |
CI MESSAGE: [2969924]: BUILD PASSED |
Signed-off-by: Kamil Tokarski ktokarski@nvidia.com
Add check against expected messages to following python assert_raises/raises tests:
dali/test/python/test_operator_squeeze.py
dali/test/python/test_dali_tf_dataset_shape.py
dali/test/python/test_external_source_impl_utils.py
dali/test/python/test_external_source_parallel.py
dali/test/python/test_operator_crop_mirror_normalize.py
dali/test/python/test_external_source_impl.py
dali/test/python/test_operator_random_object_bbox.py
Description
What happened in this PR
Additional information
Checklist
Tests
Documentation
DALI team only
Requirements
REQ IDs: N/A
JIRA TASK: DALI-2234