-
Notifications
You must be signed in to change notification settings - Fork 300
Improve error messages when comparing against objects vs strings #4928
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
rcomer
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.
Wow, thanks for the fast work @stephenworsley! I agree that trying to list out exactly what didn't match might be more trouble than it's worth, but I have a suggestion to be a bit more explicit for the coord case.
5a7af34 to
d66c697
Compare
3295b8e to
6bd15c1
Compare
rcomer
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.
Hi @stephenworsley, this looks good to me. Some very minor suggestions in line. I also wonder if it is worth updating the relevant tests to use assertRaisesRegex and check the message:
[Can't seem to find them for cube.coord !]
iris/lib/iris/tests/unit/cube/test_Cube.py
Lines 2670 to 2678 in d56824c
| def test_fail_get_cell_measure(self): | |
| with self.assertRaises(CellMeasureNotFoundError): | |
| _ = self.cube.cell_measure("notarea") | |
| def test_fail_get_cell_measures_obj(self): | |
| a_cell_measure = self.a_cell_measure.copy() | |
| a_cell_measure.units = "km2" | |
| with self.assertRaises(CellMeasureNotFoundError): | |
| _ = self.cube.cell_measure(a_cell_measure) |
iris/lib/iris/tests/unit/cube/test_Cube.py
Lines 2612 to 2620 in d56824c
| def test_fail_get_ancillary_variables(self): | |
| with self.assertRaises(AncillaryVariableNotFoundError): | |
| self.cube.ancillary_variable("other_ancill_var") | |
| def test_fail_get_ancillary_variables_obj(self): | |
| ancillary_variable = self.ancill_var.copy() | |
| ancillary_variable.long_name = "Number of observations at site" | |
| with self.assertRaises(AncillaryVariableNotFoundError): | |
| self.cube.ancillary_variable(ancillary_variable) |
785ed92 to
d83ed1c
Compare
d83ed1c to
5880aba
Compare
rcomer
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 @stephenworsley. Just waiting for the CI...
|
Seems docs are failing because https://docs.scipy.org/ is down, so the intersphinx mapping doesn't work. I'm inclined to merge anyway: the only doc change here is the whatsnew entry and it was fine before rebase. |
A simple change to error messages to address #4898.