[PLUGGABLE_BACKEND] Remove incorrect exception message matching. - #23547
Conversation
The exception comes from here: https://github.com/keras-team/keras/blob/pluggable_backend/keras/src/export/saved_model_export_archive.py#L27-L28 The message matched in the unit tests was incorrect. However, this never came up because this test only runs from trainable backends. PaddlePaddle and MLX are the first trainable backends that do not support saved_model export. Context: keras-team/keras-paddle#4
There was a problem hiding this comment.
Code Review
This pull request modifies a test assertion in keras/src/models/model_test.py to use self.assertRaises instead of self.assertRaisesRegex. The review feedback correctly points out that this violates the Keras API design guidelines, which require using self.assertRaisesRegex to verify error message clarity, and suggests restoring the regex assertion with the appropriate error message.
| r"tensorflow, jax and torch backends." | ||
| ), | ||
| ): | ||
| with self.assertRaises(NotImplementedError): |
There was a problem hiding this comment.
According to the Keras API design guidelines (under Testing -> Verify Error Clarity), we should use self.assertRaisesRegex to verify that the error message contains the expected guidance, rather than just using self.assertRaises.
Please update the assertion to use self.assertRaisesRegex with the correct expected exception message (e.g., matching "SavedModel export is not supported with backend").
| with self.assertRaises(NotImplementedError): | |
| with self.assertRaisesRegex( | |
| NotImplementedError, | |
| "SavedModel export is not supported with backend", | |
| ): |
References
- Verify Error Clarity: Use
self.assertRaisesRegexto verify that the error message contains the expected guidance. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## pluggable_backend #23547 +/- ##
====================================================
Coverage ? 84.84%
====================================================
Files ? 458
Lines ? 62462
Branches ? 10608
====================================================
Hits ? 52994
Misses ? 6781
Partials ? 2687
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The exception comes from here: https://github.com/keras-team/keras/blob/pluggable_backend/keras/src/export/saved_model_export_archive.py#L27-L28
The message matched in the unit tests was incorrect. However, this never came up because this test only runs from trainable backends. PaddlePaddle and MLX are the first trainable backends that do not support saved_model export.
Context: keras-team/keras-paddle#4
Contributor Agreement